On Tue, Jun 3, 2025 at 2:32 PM Luis Gerhorst <luis.gerho...@fau.de> wrote: > > 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. > > Signed-off-by: Luis Gerhorst <luis.gerho...@fau.de> > Acked-by: Kumar Kartikeya Dwivedi <mem...@gmail.com> > Acked-by: Henriette Herzog <henriette.her...@rub.de> > Cc: Maximilian Ott <o...@cs.fau.de> > Cc: Milan Stephan <milan.step...@fau.de> > --- > kernel/bpf/verifier.c | 89 ++++++----------- > .../selftests/bpf/progs/verifier_bounds.c | 5 +- > .../bpf/progs/verifier_bounds_deduction.c | 45 ++++++--- > .../selftests/bpf/progs/verifier_map_ptr.c | 20 +++- > .../bpf/progs/verifier_value_ptr_arith.c | 97 ++++++++++++++++--- > 5 files changed, 160 insertions(+), 96 deletions(-)
I applied the first 8, but this patch had odd merge conflicts and I didn't want to risk it. Please rebase.
