[SRU,D/E] UBUNTU: SAUCE: fs: Move SB_I_NOSUID to the top of s_iflags
diff mbox series

Message ID 20191107172153.30926-1-seth.forshee@canonical.com
State New
Headers show
Series
  • [SRU,D/E] UBUNTU: SAUCE: fs: Move SB_I_NOSUID to the top of s_iflags
Related show

Commit Message

Seth Forshee Nov. 7, 2019, 5:21 p.m. UTC
BugLink: https://bugs.launchpad.net/bugs/1851677

SB_I_NOSUID was added by a sauce patch, and over time it has come
to occpy the same bit in s_iflags as SB_I_USERNS_VISIBLE without
being noticed. overlayfs will set SB_I_NOSUID when any lower
mount is nosuid. When this happens for a user namespace mount,
mount_too_revealing() will perform additional, unnecessary checks
which may block mounting when it should be allowed.

Move SB_I_NOSUID to prevent this conflict, and move it to the top
of s_iflags to make future conflicts less likely.

Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 include/linux/fs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tyler Hicks Nov. 7, 2019, 5:34 p.m. UTC | #1
On 2019-11-07 11:21:53, Seth Forshee wrote:
> BugLink: https://bugs.launchpad.net/bugs/1851677
> 
> SB_I_NOSUID was added by a sauce patch, and over time it has come
> to occpy the same bit in s_iflags as SB_I_USERNS_VISIBLE without
> being noticed. overlayfs will set SB_I_NOSUID when any lower
> mount is nosuid. When this happens for a user namespace mount,
> mount_too_revealing() will perform additional, unnecessary checks
> which may block mounting when it should be allowed.
> 
> Move SB_I_NOSUID to prevent this conflict, and move it to the top
> of s_iflags to make future conflicts less likely.
> 
> Reported-by: Jann Horn <jannh@google.com>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

s_iflags is an unsigned long so 0x80000000 will still be within the
range even on 32 bit. This looks good to me.

 Acked-by: Tyler Hicks <tyhicks@canonical.com>

Tyler

> ---
>  include/linux/fs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 25bdf254d5c9..ff2767d66c31 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1380,12 +1380,12 @@ extern int send_sigurg(struct fown_struct *fown);
>  #define SB_I_NOEXEC	0x00000002	/* Ignore executables on this fs */
>  #define SB_I_NODEV	0x00000004	/* Ignore devices on this fs */
>  #define SB_I_MULTIROOT	0x00000008	/* Multiple roots to the dentry tree */
> -#define SB_I_NOSUID	0x00000010	/* Ignore suid on this fs */
>  
>  /* sb->s_iflags to limit user namespace mounts */
>  #define SB_I_USERNS_VISIBLE		0x00000010 /* fstype already mounted */
>  #define SB_I_IMA_UNVERIFIABLE_SIGNATURE	0x00000020
>  #define SB_I_UNTRUSTED_MOUNTER		0x00000040
> +#define SB_I_NOSUID			0x80000000 /* Ignore suid on this fs */
>  
>  /* Possible states of 'frozen' field */
>  enum {
> -- 
> 2.20.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Sultan Alsawaf Nov. 7, 2019, 5:37 p.m. UTC | #2
On Thu, Nov 07, 2019 at 11:21:53AM -0600, Seth Forshee wrote:
> BugLink: https://bugs.launchpad.net/bugs/1851677
> 
> SB_I_NOSUID was added by a sauce patch, and over time it has come
> to occpy the same bit in s_iflags as SB_I_USERNS_VISIBLE without
> being noticed. overlayfs will set SB_I_NOSUID when any lower
> mount is nosuid. When this happens for a user namespace mount,
> mount_too_revealing() will perform additional, unnecessary checks
> which may block mounting when it should be allowed.
> 
> Move SB_I_NOSUID to prevent this conflict, and move it to the top
> of s_iflags to make future conflicts less likely.
> 
> Reported-by: Jann Horn <jannh@google.com>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
>  include/linux/fs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 25bdf254d5c9..ff2767d66c31 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1380,12 +1380,12 @@ extern int send_sigurg(struct fown_struct *fown);
>  #define SB_I_NOEXEC	0x00000002	/* Ignore executables on this fs */
>  #define SB_I_NODEV	0x00000004	/* Ignore devices on this fs */
>  #define SB_I_MULTIROOT	0x00000008	/* Multiple roots to the dentry tree */
> -#define SB_I_NOSUID	0x00000010	/* Ignore suid on this fs */
>  
>  /* sb->s_iflags to limit user namespace mounts */
>  #define SB_I_USERNS_VISIBLE		0x00000010 /* fstype already mounted */
>  #define SB_I_IMA_UNVERIFIABLE_SIGNATURE	0x00000020
>  #define SB_I_UNTRUSTED_MOUNTER		0x00000040
> +#define SB_I_NOSUID			0x80000000 /* Ignore suid on this fs */
>  
>  /* Possible states of 'frozen' field */
>  enum {
> -- 
> 2.20.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

Acked-by: Sultan Alsawaf <sultan.alsawaf@canonical.com>
Khaled Elmously Nov. 8, 2019, 6:46 a.m. UTC | #3
On 2019-11-07 11:21:53 , Seth Forshee wrote:
> BugLink: https://bugs.launchpad.net/bugs/1851677
> 
> SB_I_NOSUID was added by a sauce patch, and over time it has come
> to occpy the same bit in s_iflags as SB_I_USERNS_VISIBLE without
> being noticed. overlayfs will set SB_I_NOSUID when any lower
> mount is nosuid. When this happens for a user namespace mount,
> mount_too_revealing() will perform additional, unnecessary checks
> which may block mounting when it should be allowed.
> 
> Move SB_I_NOSUID to prevent this conflict, and move it to the top
> of s_iflags to make future conflicts less likely.
> 
> Reported-by: Jann Horn <jannh@google.com>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
>  include/linux/fs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 25bdf254d5c9..ff2767d66c31 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1380,12 +1380,12 @@ extern int send_sigurg(struct fown_struct *fown);
>  #define SB_I_NOEXEC	0x00000002	/* Ignore executables on this fs */
>  #define SB_I_NODEV	0x00000004	/* Ignore devices on this fs */
>  #define SB_I_MULTIROOT	0x00000008	/* Multiple roots to the dentry tree */
> -#define SB_I_NOSUID	0x00000010	/* Ignore suid on this fs */
>  
>  /* sb->s_iflags to limit user namespace mounts */
>  #define SB_I_USERNS_VISIBLE		0x00000010 /* fstype already mounted */
>  #define SB_I_IMA_UNVERIFIABLE_SIGNATURE	0x00000020
>  #define SB_I_UNTRUSTED_MOUNTER		0x00000040
> +#define SB_I_NOSUID			0x80000000 /* Ignore suid on this fs */
>  
>  /* Possible states of 'frozen' field */
>  enum {
> -- 
> 2.20.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

Patch
diff mbox series

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 25bdf254d5c9..ff2767d66c31 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1380,12 +1380,12 @@  extern int send_sigurg(struct fown_struct *fown);
 #define SB_I_NOEXEC	0x00000002	/* Ignore executables on this fs */
 #define SB_I_NODEV	0x00000004	/* Ignore devices on this fs */
 #define SB_I_MULTIROOT	0x00000008	/* Multiple roots to the dentry tree */
-#define SB_I_NOSUID	0x00000010	/* Ignore suid on this fs */
 
 /* sb->s_iflags to limit user namespace mounts */
 #define SB_I_USERNS_VISIBLE		0x00000010 /* fstype already mounted */
 #define SB_I_IMA_UNVERIFIABLE_SIGNATURE	0x00000020
 #define SB_I_UNTRUSTED_MOUNTER		0x00000040
+#define SB_I_NOSUID			0x80000000 /* Ignore suid on this fs */
 
 /* Possible states of 'frozen' field */
 enum {