On Thu, Nov 5, 2020 at 1:55 PM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > 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 Very interesting. I was not aware of something like that nor was it mentioned when I asked on the selinux mailing list. I wonder if this is a supported feature that will always stay? In that case, NFS wouldn't need the extra hook that was added for this series. I will try this out and report back. > > > > > 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. >
