Re: [PATCH 1/1] proc01: Whitelist /proc/fs/nfsd/nfsv4recoverydir

[Jeff Layton <jlayton@kernel.org>]

On Mon, 2024-04-15 at 17:27 +0000, Chuck Lever III wrote:
> 
> > On Apr 15, 2024, at 1:21 PM, Petr Vorel <pvorel@suse.cz> wrote:
> > 
> > /proc/fs/nfsd/nfsv4recoverydir started from kernel 6.8 report EINVAL.
> > 
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > Hi,
> > 
> > @ Jeff, Chuck, Neil, NFS devs: The patch itself whitelist reading
> > /proc/fs/nfsd/nfsv4recoverydir in LTP test. I suspect reading failed
> > with EINVAL in 6.8 was a deliberate change and expected behavior when
> > CONFIG_NFSD_LEGACY_CLIENT_TRACKING is not set:
> 
> I'm not sure it was deliberate. This seems like a behavior
> regression. Jeff?
> 

I don't think I intended to make it return -EINVAL. I guess that's what
happens when there is no entry for it in the write_op array.

With CONFIG_NFSD_LEGACY_CLIENT_TRACKING disabled, that file has no
meaning or value at all anymore. Maybe we should just remove the dentry
altogether when CONFIG_NFSD_LEGACY_CLIENT_TRACKING is disabled?

> 
> > $ sudo cat /proc/fs/nfsd/nfsv4recoverydir
> > cat: /proc/fs/nfsd/nfsv4recoverydir: Invalid argument
> > 
> > I'm asking because It worked fine in kernel 6.7:
> > 
> > $ sudo cat /proc/fs/nfsd/nfsv4recoverydir
> > /var/lib/nfs/v4recovery
> > 
> > I did not bisect but I suspect suspect 74fd48739d04 ("nfsd: new Kconfig
> > option for legacy client tracking") from v6.8-rc1. The system I test
> > (openSUSE Tumbleweed) has not CONFIG_NFSD_LEGACY_CLIENT_TRACKING set and
> > 74fd48739d04 wraps write_recoverydir setup, thus it's not set.
> > 
> > +#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
> >        [NFSD_RecoveryDir] = write_recoverydir,
> > +#endif
> > 
> > Kind regards,
> > Petr
> > 
> > testcases/kernel/fs/proc/proc01.c | 1 +
> > 1 file changed, 1 insertion(+)
> > 
> > diff --git a/testcases/kernel/fs/proc/proc01.c b/testcases/kernel/fs/proc/proc01.c
> > index c90e509a3..08b9bbc75 100644
> > --- a/testcases/kernel/fs/proc/proc01.c
> > +++ b/testcases/kernel/fs/proc/proc01.c
> > @@ -113,6 +113,7 @@ static const struct mapping known_issues[] = {
> > {"read", "/proc/fs/nfsd/filehandle", EINVAL},
> > {"read", "/proc/fs/nfsd/.getfs", EINVAL},
> > {"read", "/proc/fs/nfsd/.getfd", EINVAL},
> > + {"read", "/proc/fs/nfsd/nfsv4recoverydir", EINVAL},
> > {"read", "/proc/self/net/rpc/use-gss-proxy", EAGAIN},
> > {"read", "/proc/sys/net/ipv6/conf/*/stable_secret", EIO},
> > {"read", "/proc/sys/vm/nr_hugepages", EOPNOTSUPP},
> > -- 
> > 2.43.0
> > 
> > 
> 
> --
> Chuck Lever
> 
> 

-- 
Jeff Layton <jlayton@kernel.org>