| Message ID | 20231205193250.260862-1-andreimatei1@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | bpf: fix verification of indirect var-off stack access | expand |
On Tue, 2023-12-05 at 14:32 -0500, Andrei Matei wrote: > V2 to V3: > - simplify checks for max_off (don't call > check_stack_slot_within_bounds for it) > - append a commit to protect against overflow in the addition of the > register and the offset > > V1 to V2: > - fix max_off calculation for access size = 0 > > Andrei Matei (2): > bpf: fix verification of indirect var-off stack access > bpf: guard stack limits against 32bit overflow > > kernel/bpf/verifier.c | 20 +++++++------------- > 1 file changed, 7 insertions(+), 13 deletions(-) > I think we also need a selftest, at-least for patch #1.
On Tue, Dec 5, 2023 at 3:35 PM Eduard Zingerman <eddyz87@gmail.com> wrote: > > On Tue, 2023-12-05 at 14:32 -0500, Andrei Matei wrote: > > V2 to V3: > > - simplify checks for max_off (don't call > > check_stack_slot_within_bounds for it) > > - append a commit to protect against overflow in the addition of the > > register and the offset > > > > V1 to V2: > > - fix max_off calculation for access size = 0 > > > > Andrei Matei (2): > > bpf: fix verification of indirect var-off stack access > > bpf: guard stack limits against 32bit overflow > > > > kernel/bpf/verifier.c | 20 +++++++------------- > > 1 file changed, 7 insertions(+), 13 deletions(-) > > > > I think we also need a selftest, at-least for patch #1. Also pls target bpf-next. It's a fix, but it's getting non obvious. We only push absolutely necessary fixes to bpf tree. Everything non trivial goes via bpf-next to prove itself.
On Tue, Dec 5, 2023 at 6:35 PM Eduard Zingerman <eddyz87@gmail.com> wrote: > > On Tue, 2023-12-05 at 14:32 -0500, Andrei Matei wrote: > > V2 to V3: > > - simplify checks for max_off (don't call > > check_stack_slot_within_bounds for it) > > - append a commit to protect against overflow in the addition of the > > register and the offset > > > > V1 to V2: > > - fix max_off calculation for access size = 0 > > > > Andrei Matei (2): > > bpf: fix verification of indirect var-off stack access > > bpf: guard stack limits against 32bit overflow > > > > kernel/bpf/verifier.c | 20 +++++++------------- > > 1 file changed, 7 insertions(+), 13 deletions(-) > > > > I think we also need a selftest, at-least for patch #1. Yeah... I was wondering in a message on v1 [1] if a test like what I had prototyped there was warranted. I personally still think it's not because of how many other variations of variable-offset stack access tests we already have, and how random the zero-sized read case is (even though we had a bug in there) - so protecting against this particular regression didn't seem worth it to me. I'm now including the test in v4 but if you change your mind about it when you see it in context, let me know and I'll take it out. [1] https://lore.kernel.org/bpf/CABWLsevk47Xa1a+h0UK--94zEuxScrmyt0-D8YShq1UgvVvf5g@mail.gmail.com/
On Wed, 2023-12-06 at 11:57 -0500, Andrei Matei wrote: [...] > > I think we also need a selftest, at-least for patch #1. > > Yeah... I was wondering in a message on v1 [1] if a test like what I had > prototyped there was warranted. I personally still think it's not because of > how many other variations of variable-offset stack access tests we already > have, and how random the zero-sized read case is (even though we had a bug in > there) - so protecting against this particular regression didn't seem worth it > to me. I'm now including the test in v4 but if you change your mind about it > when you see it in context, let me know and I'll take it out. > > [1] https://lore.kernel.org/bpf/CABWLsevk47Xa1a+h0UK--94zEuxScrmyt0-D8YShq1UgvVvf5g@mail.gmail.com/ The reproducer is a small and non-contrived program, so I think there is no harm in adding it.
V2 to V3: - simplify checks for max_off (don't call check_stack_slot_within_bounds for it) - append a commit to protect against overflow in the addition of the register and the offset V1 to V2: - fix max_off calculation for access size = 0 Andrei Matei (2): bpf: fix verification of indirect var-off stack access bpf: guard stack limits against 32bit overflow kernel/bpf/verifier.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)