Hi Neil, On Tue, Jun 11, 2024 at 11:03:16AM +1000, NeilBrown wrote: > On Mon, 10 Jun 2024, Jeff Layton wrote: > > On Fri, 2024-06-07 at 10:26 -0400, Mike Snitzer wrote: > > > From: Weston Andros Adamson <dros@xxxxxxxxxxxxxxx> > > > > > > Dont crash with a NULL pointer dereference when req->defer isn't > > > set. This is needed for the localio path. > > > > > > Signed-off-by: Weston Andros Adamson <dros@xxxxxxxxxxxxxxx> > > > Signed-off-by: Lance Shelton <lance.shelton@xxxxxxxxxxxxxxx> > > > Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> > > > Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx> > > > --- > > > net/sunrpc/cache.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c > > > index 95ff74706104..b757b891382c 100644 > > > --- a/net/sunrpc/cache.c > > > +++ b/net/sunrpc/cache.c > > > @@ -714,6 +714,8 @@ static bool cache_defer_req(struct cache_req > > > *req, struct cache_head *item) > > > return false; > > > } > > > > > > + if (!req->defer) > > > + return false; > > > dreq = req->defer(req); > > > if (dreq == NULL) > > > return false; > > > > I've gone over it many times, but I still don't quite "get" the > > deferral handling code. I think the above is probably safe, but please > > do Cc Neil Brown on later postings of this series since he has a better > > grasp of that code. > > -- > > Jeff Layton <jlayton@xxxxxxxxxx> > > > > The patch is bound to be "safe" in a technical sense, but I wonder why > it is necessary. And if we add code that isn't necessary we could make > the result look confusing, which isn't "safe" in a social sense... > > ->defer is always set non-NULL before svc_process() is called, and I > don't think cache_defer_req() can be reached without svc_process() being > called. So I cannot see how ->defer could possibly be NULL. > > Can you remove this patch and see if you can trigger a crash. If you > can I'd love to see the kernel stack. I removed the patch (and also the patch that exported svc_defer) and I haven't seen any issues. So I'll drop those 2 patches. Thanks, Mike
