2009-03-18 16:22:10

by J. Bruce Fields

[permalink] [raw]
Subject: nfsd patches for 2.6.29

The following bugfixes are also available from the for-2.6.29 git
repository at:

git://linux-nfs.org/~bfields/linux.git for-2.6.29

The CAP_MKNOD change should really go into CAP_FS_MASK, but that will be
a user-visible change, and some more deliberation may be required to be
sure we have CAP_FS_MASK exactly right; so I prefer to go ahead with the
one obvious nfsd-specific change now.

--b.

Benny Halevy (1):
NFSD: provide encode routine for OP_OPENATTR

J. Bruce Fields (1):
nfsd: nfsd should drop CAP_MKNOD for non-root

fs/nfsd/nfs4xdr.c | 1 +
include/linux/capability.h | 6 ++++--
2 files changed, 5 insertions(+), 2 deletions(-)


2009-03-18 16:21:51

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH] nfsd: nfsd should drop CAP_MKNOD for non-root

From: J. Bruce Fields <[email protected]>

Since creating a device node is normally an operation requiring special
privilege, Igor Zhbanov points out that it is surprising (to say the
least) that a client can, for example, create a device node on a
filesystem exported with root_squash.

So, make sure CAP_MKNOD is among the capabilities dropped when an nfsd
thread handles a request from a non-root user.

Reported-by: Igor Zhbanov <[email protected]>
Cc: [email protected]
Signed-off-by: J. Bruce Fields <[email protected]>
---
include/linux/capability.h | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/capability.h b/include/linux/capability.h
index 1b98725..4864a43 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -393,8 +393,10 @@ struct cpu_vfs_cap_data {
# define CAP_FULL_SET ((kernel_cap_t){{ ~0, ~0 }})
# define CAP_INIT_EFF_SET ((kernel_cap_t){{ ~CAP_TO_MASK(CAP_SETPCAP), ~0 }})
# define CAP_FS_SET ((kernel_cap_t){{ CAP_FS_MASK_B0, CAP_FS_MASK_B1 } })
-# define CAP_NFSD_SET ((kernel_cap_t){{ CAP_FS_MASK_B0|CAP_TO_MASK(CAP_SYS_RESOURCE), \
- CAP_FS_MASK_B1 } })
+# define CAP_NFSD_SET ((kernel_cap_t){{ CAP_FS_MASK_B0 \
+ | CAP_TO_MASK(CAP_SYS_RESOURCE) \
+ | CAP_TO_MASK(CAP_MKNOD), \
+ CAP_FS_MASK_B1 } })

#endif /* _KERNEL_CAPABILITY_U32S != 2 */

--
1.6.0.4

2009-03-18 16:22:29

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH] NFSD: provide encode routine for OP_OPENATTR

From: Benny Halevy <[email protected]>

Although this operation is unsupported by our implementation
we still need to provide an encode routine for it to
merely encode its (error) status back in the compound reply.

Thanks for Bill Baker at sun.com for testing with the Sun
OpenSolaris' client, finding, and reporting this bug at
Connectathon 2009.

This bug was introduced in 2.6.27

Signed-off-by: Benny Halevy <[email protected]>
Cc: [email protected]
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4xdr.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index f65953b..9250067 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2596,6 +2596,7 @@ static nfsd4_enc nfsd4_enc_ops[] = {
[OP_LOOKUPP] = (nfsd4_enc)nfsd4_encode_noop,
[OP_NVERIFY] = (nfsd4_enc)nfsd4_encode_noop,
[OP_OPEN] = (nfsd4_enc)nfsd4_encode_open,
+ [OP_OPENATTR] = (nfsd4_enc)nfsd4_encode_noop,
[OP_OPEN_CONFIRM] = (nfsd4_enc)nfsd4_encode_open_confirm,
[OP_OPEN_DOWNGRADE] = (nfsd4_enc)nfsd4_encode_open_downgrade,
[OP_PUTFH] = (nfsd4_enc)nfsd4_encode_noop,
--
1.6.0.4

2009-03-18 17:09:10

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [PATCH] nfsd: nfsd should drop CAP_MKNOD for non-root

Quoting J. Bruce Fields ([email protected]):
> From: J. Bruce Fields <[email protected]>
>
> Since creating a device node is normally an operation requiring special
> privilege, Igor Zhbanov points out that it is surprising (to say the
> least) that a client can, for example, create a device node on a
> filesystem exported with root_squash.
>
> So, make sure CAP_MKNOD is among the capabilities dropped when an nfsd
> thread handles a request from a non-root user.
>
> Reported-by: Igor Zhbanov <[email protected]>
> Cc: [email protected]
> Signed-off-by: J. Bruce Fields <[email protected]>

Acked-by: Serge Hallyn <[email protected]>

I assume CAP_LINUX_IMMUTABLE simply does not apply to nfs?

And, you're adding CAP_FS_MASK_B1 in anticipation of labeled nfs?

Though, I was going to send a patch later today or tomorrow (figure I
should do some ltp testing) adding CAP_MKNOD to the whole
CAP_FS_MASK_B0 (and CAP_LINUX_IMMUTABLE and CAP_FS_MASK_B1 to
CAP_FS_SET). That will conflict with this one.

thanks,
-serge

> ---
> include/linux/capability.h | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index 1b98725..4864a43 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -393,8 +393,10 @@ struct cpu_vfs_cap_data {
> # define CAP_FULL_SET ((kernel_cap_t){{ ~0, ~0 }})
> # define CAP_INIT_EFF_SET ((kernel_cap_t){{ ~CAP_TO_MASK(CAP_SETPCAP), ~0 }})
> # define CAP_FS_SET ((kernel_cap_t){{ CAP_FS_MASK_B0, CAP_FS_MASK_B1 } })
> -# define CAP_NFSD_SET ((kernel_cap_t){{ CAP_FS_MASK_B0|CAP_TO_MASK(CAP_SYS_RESOURCE), \
> - CAP_FS_MASK_B1 } })
> +# define CAP_NFSD_SET ((kernel_cap_t){{ CAP_FS_MASK_B0 \
> + | CAP_TO_MASK(CAP_SYS_RESOURCE) \
> + | CAP_TO_MASK(CAP_MKNOD), \
> + CAP_FS_MASK_B1 } })
>
> #endif /* _KERNEL_CAPABILITY_U32S != 2 */
>
> --
> 1.6.0.4

2009-03-18 17:32:38

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH] nfsd: nfsd should drop CAP_MKNOD for non-root

On Wed, Mar 18, 2009 at 12:08:43PM -0500, Serge E. Hallyn wrote:
> Quoting J. Bruce Fields ([email protected]):
> > From: J. Bruce Fields <[email protected]>
> >
> > Since creating a device node is normally an operation requiring special
> > privilege, Igor Zhbanov points out that it is surprising (to say the
> > least) that a client can, for example, create a device node on a
> > filesystem exported with root_squash.
> >
> > So, make sure CAP_MKNOD is among the capabilities dropped when an nfsd
> > thread handles a request from a non-root user.
> >
> > Reported-by: Igor Zhbanov <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: J. Bruce Fields <[email protected]>
>
> Acked-by: Serge Hallyn <[email protected]>
>
> I assume CAP_LINUX_IMMUTABLE simply does not apply to nfs?

Right. We shouldn't care how it's set.

> And, you're adding CAP_FS_MASK_B1 in anticipation of labeled nfs?

That's unchanged (would have been clearer if I hadn't re-line-wrapped in
the same patch).

> Though, I was going to send a patch later today or tomorrow (figure I
> should do some ltp testing) adding CAP_MKNOD to the whole
> CAP_FS_MASK_B0 (and CAP_LINUX_IMMUTABLE and CAP_FS_MASK_B1 to
> CAP_FS_SET). That will conflict with this one.

OK, feel free to revert this at that point if necessary.

--b.

>
> thanks,
> -serge
>
> > ---
> > include/linux/capability.h | 6 ++++--
> > 1 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/capability.h b/include/linux/capability.h
> > index 1b98725..4864a43 100644
> > --- a/include/linux/capability.h
> > +++ b/include/linux/capability.h
> > @@ -393,8 +393,10 @@ struct cpu_vfs_cap_data {
> > # define CAP_FULL_SET ((kernel_cap_t){{ ~0, ~0 }})
> > # define CAP_INIT_EFF_SET ((kernel_cap_t){{ ~CAP_TO_MASK(CAP_SETPCAP), ~0 }})
> > # define CAP_FS_SET ((kernel_cap_t){{ CAP_FS_MASK_B0, CAP_FS_MASK_B1 } })
> > -# define CAP_NFSD_SET ((kernel_cap_t){{ CAP_FS_MASK_B0|CAP_TO_MASK(CAP_SYS_RESOURCE), \
> > - CAP_FS_MASK_B1 } })
> > +# define CAP_NFSD_SET ((kernel_cap_t){{ CAP_FS_MASK_B0 \
> > + | CAP_TO_MASK(CAP_SYS_RESOURCE) \
> > + | CAP_TO_MASK(CAP_MKNOD), \
> > + CAP_FS_MASK_B1 } })
> >
> > #endif /* _KERNEL_CAPABILITY_U32S != 2 */
> >
> > --
> > 1.6.0.4

2009-03-18 20:38:16

by Igor Zhbanov

[permalink] [raw]
Subject: Re: [PATCH] nfsd: nfsd should drop CAP_MKNOD for non-root

That's good and I'm glad to see patch for CAP_NFSD_MASK in git. Thanks. :-)
Waiting for CAP_FS_MASK to be fixed too.

By the way, I don't see git repository for 2.4.x kernel. Could you fix
2.4.x too?

2009-03-18 22:28:54

by James Morris

[permalink] [raw]
Subject: Re: [PATCH] nfsd: nfsd should drop CAP_MKNOD for non-root

On Wed, 18 Mar 2009, J. Bruce Fields wrote:

> From: J. Bruce Fields <[email protected]>
>
> Since creating a device node is normally an operation requiring special
> privilege, Igor Zhbanov points out that it is surprising (to say the
> least) that a client can, for example, create a device node on a
> filesystem exported with root_squash.
>
> So, make sure CAP_MKNOD is among the capabilities dropped when an nfsd
> thread handles a request from a non-root user.
>
> Reported-by: Igor Zhbanov <[email protected]>
> Cc: [email protected]
> Signed-off-by: J. Bruce Fields <[email protected]>

Acked-by: James Morris <[email protected]>

> ---
> include/linux/capability.h | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index 1b98725..4864a43 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -393,8 +393,10 @@ struct cpu_vfs_cap_data {
> # define CAP_FULL_SET ((kernel_cap_t){{ ~0, ~0 }})
> # define CAP_INIT_EFF_SET ((kernel_cap_t){{ ~CAP_TO_MASK(CAP_SETPCAP), ~0 }})
> # define CAP_FS_SET ((kernel_cap_t){{ CAP_FS_MASK_B0, CAP_FS_MASK_B1 } })
> -# define CAP_NFSD_SET ((kernel_cap_t){{ CAP_FS_MASK_B0|CAP_TO_MASK(CAP_SYS_RESOURCE), \
> - CAP_FS_MASK_B1 } })
> +# define CAP_NFSD_SET ((kernel_cap_t){{ CAP_FS_MASK_B0 \
> + | CAP_TO_MASK(CAP_SYS_RESOURCE) \
> + | CAP_TO_MASK(CAP_MKNOD), \
> + CAP_FS_MASK_B1 } })
>
> #endif /* _KERNEL_CAPABILITY_U32S != 2 */
>
> --
> 1.6.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

--
James Morris
<[email protected]>