diff mbox series

[v5,1/1] NFSv4.2: condition READDIR's mask for security label based on LSM state

Message ID 20201106210338.5032-1-olga.kornievskaia@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v5,1/1] NFSv4.2: condition READDIR's mask for security label based on LSM state | expand

Commit Message

Olga Kornievskaia Nov. 6, 2020, 9:03 p.m. UTC 
From: Olga Kornievskaia <kolga@netapp.com>

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.

Client adjusts superblock's support of the security_label based on
the server's support but also current client's configuration of the
LSM modules. Thus, prior to using the default bitmask in READDIR,
this patch checks the server's capabilities and then instructs
READDIR to remove FATTR4_WORD2_SECURITY_LABEL from the bitmask.

v5: fixing silly mistakes of the rushed v4
v4: simplifying logic
v3: changing label's initialization per Ondrej's comment
v2: dropping selinux hook and using the sb cap.

Suggested-by: Ondrej Mosnacek <omosnace@redhat.com>
Suggested-by: Scott Mayhew <smayhew@redhat.com>
Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs4proc.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Trond Myklebust Nov. 7, 2020, 12:16 a.m. UTC | #1 
On Fri, 2020-11-06 at 16:03 -0500, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <kolga@netapp.com>
> 
> 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.
> 
> Client adjusts superblock's support of the security_label based on
> the server's support but also current client's configuration of the
> LSM modules. Thus, prior to using the default bitmask in READDIR,
> this patch checks the server's capabilities and then instructs
> READDIR to remove FATTR4_WORD2_SECURITY_LABEL from the bitmask.
> 
> v5: fixing silly mistakes of the rushed v4
> v4: simplifying logic
> v3: changing label's initialization per Ondrej's comment
> v2: dropping selinux hook and using the sb cap.
> 
> Suggested-by: Ondrej Mosnacek <omosnace@redhat.com>
> Suggested-by: Scott Mayhew <smayhew@redhat.com>
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  fs/nfs/nfs4proc.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 9e0ca9b2b210..7fa63e282af0 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -4961,12 +4961,12 @@ static int _nfs4_proc_readdir(struct dentry
> *dentry, const struct cred *cred,
>                 u64 cookie, struct page **pages, unsigned int count,
> bool plus)
>  {
>         struct inode            *dir = d_inode(dentry);
> +       struct nfs_server       *server = NFS_SERVER(dir);
>         struct nfs4_readdir_arg args = {
>                 .fh = NFS_FH(dir),
>                 .pages = pages,
>                 .pgbase = 0,
>                 .count = count,
> -               .bitmask = NFS_SERVER(d_inode(dentry))->attr_bitmask,
>                 .plus = plus,
>         };
>         struct nfs4_readdir_res res;
> @@ -4981,9 +4981,15 @@ static int _nfs4_proc_readdir(struct dentry
> *dentry, const struct cred *cred,
>         dprintk("%s: dentry = %pd2, cookie = %Lu\n", __func__,
>                         dentry,
>                         (unsigned long long)cookie);
> +       if (!(server->caps & NFS_CAP_SECURITY_LABEL))
> +               args.bitmask = server->attr_bitmask_nl;
> +       else
> +               args.bitmask = server->attr_bitmask;
> +
>         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);
> +       status = nfs4_call_sync(server->client, server, &msg,
> &args.seq_args,
> +                       &res.seq_res, 0);
>         if (status >= 0) {
>                 memcpy(NFS_I(dir)->cookieverf, res.verifier.data,
> NFS4_VERIFIER_SIZE);
>                 status += args.pgbase;

That version looks good to me. Thanks Olga!
diff mbox series

Patch

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 9e0ca9b2b210..7fa63e282af0 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4961,12 +4961,12 @@  static int _nfs4_proc_readdir(struct dentry *dentry, const struct cred *cred,
 		u64 cookie, struct page **pages, unsigned int count, bool plus)
 {
 	struct inode		*dir = d_inode(dentry);
+	struct nfs_server	*server = NFS_SERVER(dir);
 	struct nfs4_readdir_arg args = {
 		.fh = NFS_FH(dir),
 		.pages = pages,
 		.pgbase = 0,
 		.count = count,
-		.bitmask = NFS_SERVER(d_inode(dentry))->attr_bitmask,
 		.plus = plus,
 	};
 	struct nfs4_readdir_res res;
@@ -4981,9 +4981,15 @@  static int _nfs4_proc_readdir(struct dentry *dentry, const struct cred *cred,
 	dprintk("%s: dentry = %pd2, cookie = %Lu\n", __func__,
 			dentry,
 			(unsigned long long)cookie);
+	if (!(server->caps & NFS_CAP_SECURITY_LABEL))
+		args.bitmask = server->attr_bitmask_nl;
+	else
+		args.bitmask = server->attr_bitmask;
+
 	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);
+	status = nfs4_call_sync(server->client, server, &msg, &args.seq_args,
+			&res.seq_res, 0);
 	if (status >= 0) {
 		memcpy(NFS_I(dir)->cookieverf, res.verifier.data, NFS4_VERIFIER_SIZE);
 		status += args.pgbase;