bpf: Fall back to nospec for sanitization-failures

[luisgerhorst]

ALU sanitization was introduced to ensure that a subsequent ptr access can never go OOB, even under speculation. This is required because we currently allow speculative scalar confusion. Spec. scalar confusion is possible because Spectre v4 sanitization only adds a nospec after critical stores (e.g., scalar overwritten with a pointer).

If we add a nospec before the ALU op, none of the operands can be subject to scalar confusion. As an ADD/SUB can not introduce scalar confusion itself, the result will also not be subject to scalar confusion. Therefore, the subsequent ptr access is always safe.

We directly fall back to nospec for the sanitization errors REASON_BOUNDS, _TYPE, _PATHS, and _LIMIT, even if we are not on a speculative path.

For REASON_STACK, we return the error -ENOMEM directly now. Previously, sanitize_err() returned -EACCES for this case but we change it to -ENOMEM because doing so prevents do_check() from falling back to a nospec if we are on a speculative path. This would not be a serious issue (the verifier would probably run into the -ENOMEM again shortly on the next non-speculative path and still abort verification), but -ENOMEM is more fitting here anyway. An alternative would be -EFAULT, which is also returned for some of the other cases where push_stack() fails, but this is more frequently used for verifier-internal bugs.

Regarding 97744b4 ("bpf: Clarify sanitize_check_bounds()"), two cases are relevant.

• First, if a case in sanitize_check_bounds() is omitted, it fails with EOPNOTSUPP but retrieve_ptr_limit() returns 0 (thereby potentially not setting cur_aux(env)->nospec), TODO therefore the verifier bug is detected as expected?

• Second, if a case is omitted from retrieve_ptr_limit() but not from sanitize_check_bounds(), bounds_ret equals 0 or -EACCES. If it is 0 and the default case in retrieve_ptr_limit() runs errorously, we mark the insn for nospec-sanitization. This is not really a security problem and it could also only be detected by adding a verifier_bug_if(bounds_ret != -EOPNOTSUPP) into the default case in retrieve_ptr_limit().

Signed-off-by: Luis Gerhorst [email protected]
Acked-by: Kumar Kartikeya Dwivedi [email protected]
Acked-by: Henriette Herzog [email protected]
Cc: Maximilian Ott [email protected]
Cc: Milan Stephan [email protected]

Changes since v4 of series:

• Rebase and resolve merge conflicts with 97744b4 ("bpf: Clarify sanitize_check_bounds()").
• Change update_alu_sanitation_state() and sanitize_val_alu() to return void as they now never fail.
• Link to v4 of series: https://lore.kernel.org/all/[email protected]/