On Thu, Nov 5, 2020 at 6:33 PM Olga Kornievskaia <olga.kornievskaia@xxxxxxxxx> wrote: > From: Olga Kornievskaia <kolga@xxxxxxxxxx> > > Currently, the client will always ask for security_labels if the server > returns that it supports that feature regardless of any LSM modules > (such as Selinux) enforcing security policy. This adds performance > penalty to the READDIR operation. > > Instead, query the LSM module to find if anything is enabled and > if not, then remove FATTR4_WORD2_SECURITY_LABEL from the bitmask. Having spent some time staring at some of the NFS code very recently, I can't help but suggest: Would it perhaps be enough to decide whether to ask for labels based on (NFS_SB(dentry->d_sb)->caps & NFS_CAP_SECURITY_LABEL)? It is set when mounting the FS iff some LSM confirms via the security_sb_*_mnt_opts() hook that it wants the filesystem to give it labels (or at least that's how I interpret the cryptic name) [1]. It's just a shot in the dark, but it seems to fit this use case. [1] https://elixir.bootlin.com/linux/v5.10-rc2/source/fs/nfs/getroot.c#L148 > > Suggested-by: Scott Mayhew <smayhew@xxxxxxxxxx> > Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx> > --- > fs/nfs/nfs4proc.c | 5 +++++ > fs/nfs/nfs4xdr.c | 3 ++- > include/linux/nfs_xdr.h | 1 + > 3 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > index 9e0ca9b2b210..774bc5e63ca7 100644 > --- a/fs/nfs/nfs4proc.c > +++ b/fs/nfs/nfs4proc.c > @@ -55,6 +55,7 @@ > #include <linux/utsname.h> > #include <linux/freezer.h> > #include <linux/iversion.h> > +#include <linux/security.h> > > #include "nfs4_fs.h" > #include "delegation.h" > @@ -4968,6 +4969,7 @@ static int _nfs4_proc_readdir(struct dentry *dentry, const struct cred *cred, > .count = count, > .bitmask = NFS_SERVER(d_inode(dentry))->attr_bitmask, > .plus = plus, > + .labels = true, > }; > struct nfs4_readdir_res res; > struct rpc_message msg = { > @@ -4977,10 +4979,13 @@ static int _nfs4_proc_readdir(struct dentry *dentry, const struct cred *cred, > .rpc_cred = cred, > }; > int status; > + int sec_flags = LSM_FQUERY_VFS_XATTRS; > > dprintk("%s: dentry = %pd2, cookie = %Lu\n", __func__, > dentry, > (unsigned long long)cookie); > + if (!security_func_query_vfs(sec_flags)) > + args.labels = false; > nfs4_setup_readdir(cookie, NFS_I(dir)->cookieverf, dentry, &args); > res.pgbase = args.pgbase; > status = nfs4_call_sync(NFS_SERVER(dir)->client, NFS_SERVER(dir), &msg, &args.seq_args, &res.seq_res, 0); > diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c > index c6dbfcae7517..585d5b5cc3dc 100644 > --- a/fs/nfs/nfs4xdr.c > +++ b/fs/nfs/nfs4xdr.c > @@ -1605,7 +1605,8 @@ static void encode_readdir(struct xdr_stream *xdr, const struct nfs4_readdir_arg > FATTR4_WORD1_OWNER_GROUP|FATTR4_WORD1_RAWDEV| > FATTR4_WORD1_SPACE_USED|FATTR4_WORD1_TIME_ACCESS| > FATTR4_WORD1_TIME_METADATA|FATTR4_WORD1_TIME_MODIFY; > - attrs[2] |= FATTR4_WORD2_SECURITY_LABEL; > + if (readdir->labels) > + attrs[2] |= FATTR4_WORD2_SECURITY_LABEL; > dircount >>= 1; > } > /* Use mounted_on_fileid only if the server supports it */ > diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h > index d63cb862d58e..95f648b26525 100644 > --- a/include/linux/nfs_xdr.h > +++ b/include/linux/nfs_xdr.h > @@ -1119,6 +1119,7 @@ struct nfs4_readdir_arg { > unsigned int pgbase; /* zero-copy data */ > const u32 * bitmask; > bool plus; > + bool labels; > }; > > struct nfs4_readdir_res { > -- > 2.18.2 > -- Ondrej Mosnacek Software Engineer, Platform Security - SELinux kernel Red Hat, Inc.
