| Message ID | 20220715183257.41129-1-jlayton@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | nfsd: close potential race between open and delegation | expand |
> On Jul 15, 2022, at 2:32 PM, Jeff Layton <jlayton@kernel.org> wrote: > > v2: > - use nfsd_lookup_dentry instead of lookup_one_len > > Here's a respin of the patches to fix up the potential race between an > open and delegation. I took Neil's advice an changed over to use > nfsd_lookup_dentry. > > This patchset is based on top of Neil's recent patchset entitled: > > [PATCH 0/8] NFSD: clean up locking. Thanks to both of you for pursuing this work! I think there are some good improvements here. Note that there are some outstanding review comments (aside from the disagreement about how to refactor nfsd_create) so I expect Neil will be reposting his series. This is just to note that, as long as your series is based on his, I will consider your series as RFC until his base series is stable and pulled. I'll review again today or over the weekend. > Tested with xfstests and it seemed to behave. I haven't done any testing > to ensure that the race is actually fixed, mainly because I don't have a > way to reliably reproduce it. That's the thing: we don't have many tests that use multiple clients targeting the same set of files. > Jeff Layton (2): > nfsd: drop fh argument from alloc_init_deleg > nfsd: vet the opened dentry after setting a delegation > > fs/nfsd/nfs4state.c | 58 ++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 49 insertions(+), 9 deletions(-) > > -- > 2.36.1 > -- Chuck Lever
On Fri, 2022-07-15 at 18:42 +0000, Chuck Lever III wrote: > > > On Jul 15, 2022, at 2:32 PM, Jeff Layton <jlayton@kernel.org> wrote: > > > > v2: > > - use nfsd_lookup_dentry instead of lookup_one_len > > > > Here's a respin of the patches to fix up the potential race between an > > open and delegation. I took Neil's advice an changed over to use > > nfsd_lookup_dentry. > > > > This patchset is based on top of Neil's recent patchset entitled: > > > > [PATCH 0/8] NFSD: clean up locking. > > Thanks to both of you for pursuing this work! I think there are > some good improvements here. > > Note that there are some outstanding review comments (aside from > the disagreement about how to refactor nfsd_create) so I expect > Neil will be reposting his series. This is just to note that, as > long as your series is based on his, I will consider your series > as RFC until his base series is stable and pulled. > > I'll review again today or over the weekend. > Thanks. I think I'll be able to adapt this approach on top of whatever Neil comes up with. > > > Tested with xfstests and it seemed to behave. I haven't done any testing > > to ensure that the race is actually fixed, mainly because I don't have a > > way to reliably reproduce it. > > That's the thing: we don't have many tests that use multiple clients > targeting the same set of files. > > Yeah, it's a difficult problem. Testing delegation behavior is particularly difficult since the client doesn't have a lot of control over them being granted in the first place. > > Jeff Layton (2): > > nfsd: drop fh argument from alloc_init_deleg > > nfsd: vet the opened dentry after setting a delegation > > > > fs/nfsd/nfs4state.c | 58 ++++++++++++++++++++++++++++++++++++++------- > > 1 file changed, 49 insertions(+), 9 deletions(-) > > > > -- > > 2.36.1 > > > > -- > Chuck Lever > > >
v2: - use nfsd_lookup_dentry instead of lookup_one_len Here's a respin of the patches to fix up the potential race between an open and delegation. I took Neil's advice an changed over to use nfsd_lookup_dentry. This patchset is based on top of Neil's recent patchset entitled: [PATCH 0/8] NFSD: clean up locking. Tested with xfstests and it seemed to behave. I haven't done any testing to ensure that the race is actually fixed, mainly because I don't have a way to reliably reproduce it. Jeff Layton (2): nfsd: drop fh argument from alloc_init_deleg nfsd: vet the opened dentry after setting a delegation fs/nfsd/nfs4state.c | 58 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 49 insertions(+), 9 deletions(-)