On Mon, Aug 26, 2024 at 10:47:46AM -0400, Jeff Layton wrote: > On Mon, 2024-08-26 at 10:37 -0400, Jeff Layton wrote: > > On Mon, 2024-08-26 at 09:22 +1000, NeilBrown wrote: > > > On Sun, 25 Aug 2024, Chuck Lever wrote: > > > > On Fri, Aug 23, 2024 at 06:27:37PM -0400, Jeff Layton wrote: > > > > > Fixes for a couple of CB_GETATTR bugs I found while working on the > > > > > delstid set. Mostly this just ensures that we hold references to the > > > > > delegation while working with it. > > > > > > > > > > > > > > > > > > Applied to nfsd-fixes for v6.11-rc, thanks! > > > > > > > > [1/2] nfsd: hold reference to delegation when updating it for cb_getattr > > > > commit: 8fceb5f6636bbbf803fe29fff59f138206559964 > > > > [2/2] nfsd: fix potential UAF in nfsd4_cb_getattr_release > > > > commit: 8bc97f9b84c8852fcc56be2382f5115c518de785 > > > > > > > > -- > > > > Chuck Lever > > > > > > > > > > Maybe the following can tidy up that code. I can split this into > > > a few separate patches if you like. > > > Thoughts? > > > > > > Note that the patch is easier to review if you apply it then use "git > > > diff -b". > > > > > > NeilBrown > > > > > > > > > From: NeilBrown <neilb@xxxxxxx> > > > Subject: [PATCH] nfsd: untangle code in nfsd4_deleg_getattr_conflict() > > > > > > The code in nfsd4_deleg_getattr_conflict() is convoluted and buggy. > > > > > > With this patch we: > > > - properly handle non-nfsd leases. We must not assume flc_owner is a > > > delegation unless fl_lmops == &nfsd_lease_mng_ops > > > > AFAICT, non-nfsd leases are already properly handled (though I do agree > > that the "flow" of this code is awkward). What case do you see that's > > wrong? > > > > Doh! Nevermind -- I see it now. It looks like the break_lease tag is > just in the wrong place. We should definitely fix that. > > In any case, your patch looks reasonable to me, but I couldn't get it > to apply. I applied Jeff's weekend CB_GETATTR patches to nfsd-fixes. If there's an additional bug fix carried in Neil's clean-up, I would like that to apply to that branch, as a small surgical fix, so it can go into v6.11-rc. Seems like these CB_GETATTR fixes need to be applicable to LTS kernels, so let's keep them narrow. > Care to send a real PATCH instead? It's fine if you want to > drop my patch and just replace it with yours. Neil, I'd prefer: 1) specific fixes to apply to the nfsd-fixes branch 2) larger clean-ups to apply to the nfsd-next branch Untangling nfsd4_deleg_getattr_conflict() is a sensible thing to do, IMO, but I'd bet that Linus would consider that development rather than an urgent bug fix. -- Chuck Lever
