Re: [Patch 6/6] Bind Mount Extensions 0.06
On Wed, Feb 23, 2005 at 11:06:59PM +, Christoph Hellwig wrote:
> > +++
> > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/ext2/ioctl.c
> > 2005-02-19 06:32:05 +0100
> > @@ -29,7 +29,8 @@ int ext2_ioctl (struct inode * inode, st
> > case EXT2_IOC_SETFLAGS: {
> > unsigned int oldflags;
> >
> > - if (IS_RDONLY(inode))
> > + if (IS_RDONLY(inode) ||
> > + (filp && MNT_IS_RDONLY(filp->f_vfsmnt)))
>
> doing this in every filesystem ->ioctl is a really bad idea. We need to
> add common handling for ext2-style file attributes first.
hmm, well, but the ioctls are somewhat mixed, i.e.
some of them do just read (only) like operations,
others do change stuff, and the test is just valid
for write/change ioctls ...
of course I could add a second switch/case block
which checks for 'write' type ioctls and blocks
them in the beginning ...
but maybe I did misunderstood your comment, so let
me know what you consider appropriate ...
> Also please add a file_readonly() helper - when introduced it only checks
> IS_RDONLY(file->f_dentry->d_inode) and once you add per-mount flags it
> only needs to be added in a single place. Actually probably a lowelevel
> one taking inode,vfsmount and wrappers for a struct file * or
> struct nameidata * which would cover most of the cases.
actually I started the BME patches by extending the
IS_RDONLY() macro to take two arguments, the inode
and the vfsmount (which sounded natural to me) but
that was shot down ... (don't remember why exactly)
no problem with a file_readonly() or nd_readonly()
if that makes folks happy ...
thanks,
Herbert
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 6/6] Bind Mount Extensions 0.06
On Wed, Feb 23, 2005 at 11:06:59PM +, Christoph Hellwig wrote:
+++
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/ext2/ioctl.c
2005-02-19 06:32:05 +0100
@@ -29,7 +29,8 @@ int ext2_ioctl (struct inode * inode, st
case EXT2_IOC_SETFLAGS: {
unsigned int oldflags;
- if (IS_RDONLY(inode))
+ if (IS_RDONLY(inode) ||
+ (filp MNT_IS_RDONLY(filp-f_vfsmnt)))
doing this in every filesystem -ioctl is a really bad idea. We need to
add common handling for ext2-style file attributes first.
hmm, well, but the ioctls are somewhat mixed, i.e.
some of them do just read (only) like operations,
others do change stuff, and the test is just valid
for write/change ioctls ...
of course I could add a second switch/case block
which checks for 'write' type ioctls and blocks
them in the beginning ...
but maybe I did misunderstood your comment, so let
me know what you consider appropriate ...
Also please add a file_readonly() helper - when introduced it only checks
IS_RDONLY(file-f_dentry-d_inode) and once you add per-mount flags it
only needs to be added in a single place. Actually probably a lowelevel
one taking inode,vfsmount and wrappers for a struct file * or
struct nameidata * which would cover most of the cases.
actually I started the BME patches by extending the
IS_RDONLY() macro to take two arguments, the inode
and the vfsmount (which sounded natural to me) but
that was shot down ... (don't remember why exactly)
no problem with a file_readonly() or nd_readonly()
if that makes folks happy ...
thanks,
Herbert
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 6/6] Bind Mount Extensions 0.06
> +++
> linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/ext2/ioctl.c
> 2005-02-19 06:32:05 +0100
> @@ -29,7 +29,8 @@ int ext2_ioctl (struct inode * inode, st
> case EXT2_IOC_SETFLAGS: {
> unsigned int oldflags;
>
> - if (IS_RDONLY(inode))
> + if (IS_RDONLY(inode) ||
> + (filp && MNT_IS_RDONLY(filp->f_vfsmnt)))
doing this in every filesystem ->ioctl is a really bad idea. We need to
add common handling for ext2-style file attributes first.
Also please add a file_readonly() helper - when introduced it only checks
IS_RDONLY(file->f_dentry->d_inode) and once you add per-mount flags it
only needs to be added in a single place. Actually probably a lowelevel
one taking inode,vfsmount and wrappers for a struct file * or
struct nameidata * which would cover most of the cases.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 6/6] Bind Mount Extensions 0.06
On Tue, Feb 22, 2005 at 09:58:45AM -0500, Trond Myklebust wrote:
> ty den 22.02.2005 Klokka 13:13 (+0100) skreiv Herbert Poetzl:
>
> > diff -NurpP --minimal
> > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/arch/sparc64/solaris/fs.c
> >
> > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/arch/sparc64/solaris/fs.c
> > ---
> > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/arch/sparc64/solaris/fs.c
> >2004-12-25 01:54:50 +0100
> > +++
> > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/arch/sparc64/solaris/fs.c
> > 2005-02-19 06:32:05 +0100
> > @@ -362,7 +362,7 @@ static int report_statvfs(struct vfsmoun
> > int j = strlen (p);
> >
> > if (j > 15) j = 15;
> > - if (IS_RDONLY(inode)) i = 1;
> > + if (IS_RDONLY(inode) || (mnt && MNT_IS_RDONLY(mnt))) i = 1;
>
> Redundant check of mnt != NULL.
yep,
> > if (mnt->mnt_flags & MNT_NOSUID) i |= 2;
> > if (!sysv_valid_dev(inode->i_sb->s_dev))
> > return -EOVERFLOW;
> > @@ -398,7 +398,7 @@ static int report_statvfs64(struct vfsmo
> > int j = strlen (p);
> >
> > if (j > 15) j = 15;
> > - if (IS_RDONLY(inode)) i = 1;
> > + if (IS_RDONLY(inode) || (mnt && MNT_IS_RDONLY(mnt))) i = 1;
>
> Redundant check of mnt != NULL
agreed, thanks!
> > if (mnt->mnt_flags & MNT_NOSUID) i |= 2;
> > if (!sysv_valid_dev(inode->i_sb->s_dev))
> > return -EOVERFLOW;
> > diff -NurpP --minimal
> > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/namei.c
> > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/namei.c
> > --- linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/namei.c
> > 2005-02-19 06:31:50 +0100
> > +++
> > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/namei.c
> >2005-02-19 06:32:05 +0100
> > @@ -220,7 +220,7 @@ int permission(struct inode *inode, int
> > /*
> > * Nobody gets write access to a read-only fs.
> > */
> > - if (IS_RDONLY(inode) &&
> > + if ((IS_RDONLY(inode) || (nd && MNT_IS_RDONLY(nd->mnt))) &&
> > (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
> > return -EROFS;
>
> This is very dodgy. What if the user is calling permission without
> setting the (currently optional) nameidata hint? Have you audited the
> kernel to find out if this is safe?
safe yes, aybe not 'correct' I agree that moving
the check 'upwards' into the callers might be
the better solution here, will look into it ...
> > diff -NurpP --minimal
> > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/open.c
> > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/open.c
> > --- linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/open.c
> > 2005-02-19 06:31:43 +0100
> > +++
> > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/open.c
> > 2005-02-19 06:32:05 +0100
> > @@ -239,7 +239,7 @@ static inline long do_sys_truncate(const
> > goto dput_and_out;
> >
> > error = -EROFS;
> > - if (IS_RDONLY(inode))
> > + if (IS_RDONLY(inode) || MNT_IS_RDONLY(nd.mnt))
> > goto dput_and_out;
> >
> > error = -EPERM;
> > @@ -363,7 +363,7 @@ asmlinkage long sys_utime(char __user *
> > inode = nd.dentry->d_inode;
> >
> > error = -EROFS;
> > - if (IS_RDONLY(inode))
> > + if (IS_RDONLY(inode) || MNT_IS_RDONLY(nd.mnt))
> > goto dput_and_out;
> >
> > /* Don't worry, the checks are done in inode_change_ok() */
> > @@ -420,7 +420,7 @@ long do_utimes(char __user * filename, s
> > inode = nd.dentry->d_inode;
> >
> > error = -EROFS;
> > - if (IS_RDONLY(inode))
> > + if (IS_RDONLY(inode) || MNT_IS_RDONLY(nd.mnt))
> > goto dput_and_out;
> >
> > /* Don't worry, the checks are done in inode_change_ok() */
> > @@ -502,7 +502,8 @@ asmlinkage long sys_access(const char __
> > if (!res) {
> > res = permission(nd.dentry->d_inode, mode, );
> > /* SuS v2 requires we report a read only fs too */
> > - if(!res && (mode & S_IWOTH) && IS_RDONLY(nd.dentry->d_inode)
> > + if(!res && (mode & S_IWOTH)
> > + && (IS_RDONLY(nd.dentry->d_inode) || MNT_IS_RDONLY(nd.mnt))
> >&& !special_file(nd.dentry->d_inode->i_mode))
> > res = -EROFS;
> > path_release();
> > @@ -608,7 +609,7 @@ asmlinkage long sys_fchmod(unsigned int
> > inode = dentry->d_inode;
> >
> > err = -EROFS;
> > - if (IS_RDONLY(inode))
> > + if (IS_RDONLY(inode) || (file && MNT_IS_RDONLY(file->f_vfsmnt)))
> > goto out_putf;
>
> Redundant check of file != NULL.
ack!
> > err = -EPERM;
> > if (IS_IMMUTABLE(inode) ||
Re: [Patch 6/6] Bind Mount Extensions 0.06
On Tue, Feb 22, 2005 at 09:58:45AM -0500, Trond Myklebust wrote:
ty den 22.02.2005 Klokka 13:13 (+0100) skreiv Herbert Poetzl:
diff -NurpP --minimal
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/arch/sparc64/solaris/fs.c
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/arch/sparc64/solaris/fs.c
---
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/arch/sparc64/solaris/fs.c
2004-12-25 01:54:50 +0100
+++
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/arch/sparc64/solaris/fs.c
2005-02-19 06:32:05 +0100
@@ -362,7 +362,7 @@ static int report_statvfs(struct vfsmoun
int j = strlen (p);
if (j 15) j = 15;
- if (IS_RDONLY(inode)) i = 1;
+ if (IS_RDONLY(inode) || (mnt MNT_IS_RDONLY(mnt))) i = 1;
Redundant check of mnt != NULL.
yep,
if (mnt-mnt_flags MNT_NOSUID) i |= 2;
if (!sysv_valid_dev(inode-i_sb-s_dev))
return -EOVERFLOW;
@@ -398,7 +398,7 @@ static int report_statvfs64(struct vfsmo
int j = strlen (p);
if (j 15) j = 15;
- if (IS_RDONLY(inode)) i = 1;
+ if (IS_RDONLY(inode) || (mnt MNT_IS_RDONLY(mnt))) i = 1;
Redundant check of mnt != NULL
agreed, thanks!
if (mnt-mnt_flags MNT_NOSUID) i |= 2;
if (!sysv_valid_dev(inode-i_sb-s_dev))
return -EOVERFLOW;
diff -NurpP --minimal
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/namei.c
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/namei.c
--- linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/namei.c
2005-02-19 06:31:50 +0100
+++
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/namei.c
2005-02-19 06:32:05 +0100
@@ -220,7 +220,7 @@ int permission(struct inode *inode, int
/*
* Nobody gets write access to a read-only fs.
*/
- if (IS_RDONLY(inode)
+ if ((IS_RDONLY(inode) || (nd MNT_IS_RDONLY(nd-mnt)))
(S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
return -EROFS;
This is very dodgy. What if the user is calling permission without
setting the (currently optional) nameidata hint? Have you audited the
kernel to find out if this is safe?
safe yes, aybe not 'correct' I agree that moving
the check 'upwards' into the callers might be
the better solution here, will look into it ...
diff -NurpP --minimal
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/open.c
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/open.c
--- linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/open.c
2005-02-19 06:31:43 +0100
+++
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/open.c
2005-02-19 06:32:05 +0100
@@ -239,7 +239,7 @@ static inline long do_sys_truncate(const
goto dput_and_out;
error = -EROFS;
- if (IS_RDONLY(inode))
+ if (IS_RDONLY(inode) || MNT_IS_RDONLY(nd.mnt))
goto dput_and_out;
error = -EPERM;
@@ -363,7 +363,7 @@ asmlinkage long sys_utime(char __user *
inode = nd.dentry-d_inode;
error = -EROFS;
- if (IS_RDONLY(inode))
+ if (IS_RDONLY(inode) || MNT_IS_RDONLY(nd.mnt))
goto dput_and_out;
/* Don't worry, the checks are done in inode_change_ok() */
@@ -420,7 +420,7 @@ long do_utimes(char __user * filename, s
inode = nd.dentry-d_inode;
error = -EROFS;
- if (IS_RDONLY(inode))
+ if (IS_RDONLY(inode) || MNT_IS_RDONLY(nd.mnt))
goto dput_and_out;
/* Don't worry, the checks are done in inode_change_ok() */
@@ -502,7 +502,8 @@ asmlinkage long sys_access(const char __
if (!res) {
res = permission(nd.dentry-d_inode, mode, nd);
/* SuS v2 requires we report a read only fs too */
- if(!res (mode S_IWOTH) IS_RDONLY(nd.dentry-d_inode)
+ if(!res (mode S_IWOTH)
+ (IS_RDONLY(nd.dentry-d_inode) || MNT_IS_RDONLY(nd.mnt))
!special_file(nd.dentry-d_inode-i_mode))
res = -EROFS;
path_release(nd);
@@ -608,7 +609,7 @@ asmlinkage long sys_fchmod(unsigned int
inode = dentry-d_inode;
err = -EROFS;
- if (IS_RDONLY(inode))
+ if (IS_RDONLY(inode) || (file MNT_IS_RDONLY(file-f_vfsmnt)))
goto out_putf;
Redundant check of file != NULL.
ack!
err = -EPERM;
if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
@@ -640,7 +641,7 @@ asmlinkage long sys_chmod(const char __u
inode = nd.dentry-d_inode;
error = -EROFS;
- if (IS_RDONLY(inode))
+ if (IS_RDONLY(inode) || MNT_IS_RDONLY(nd.mnt))
goto dput_and_out;
Re: [Patch 6/6] Bind Mount Extensions 0.06
+++
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/ext2/ioctl.c
2005-02-19 06:32:05 +0100
@@ -29,7 +29,8 @@ int ext2_ioctl (struct inode * inode, st
case EXT2_IOC_SETFLAGS: {
unsigned int oldflags;
- if (IS_RDONLY(inode))
+ if (IS_RDONLY(inode) ||
+ (filp MNT_IS_RDONLY(filp-f_vfsmnt)))
doing this in every filesystem -ioctl is a really bad idea. We need to
add common handling for ext2-style file attributes first.
Also please add a file_readonly() helper - when introduced it only checks
IS_RDONLY(file-f_dentry-d_inode) and once you add per-mount flags it
only needs to be added in a single place. Actually probably a lowelevel
one taking inode,vfsmount and wrappers for a struct file * or
struct nameidata * which would cover most of the cases.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 6/6] Bind Mount Extensions 0.06
ty den 22.02.2005 Klokka 13:13 (+0100) skreiv Herbert Poetzl:
> diff -NurpP --minimal
> linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/arch/sparc64/solaris/fs.c
>
> linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/arch/sparc64/solaris/fs.c
> ---
> linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/arch/sparc64/solaris/fs.c
> 2004-12-25 01:54:50 +0100
> +++
> linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/arch/sparc64/solaris/fs.c
> 2005-02-19 06:32:05 +0100
> @@ -362,7 +362,7 @@ static int report_statvfs(struct vfsmoun
> int j = strlen (p);
>
> if (j > 15) j = 15;
> - if (IS_RDONLY(inode)) i = 1;
> + if (IS_RDONLY(inode) || (mnt && MNT_IS_RDONLY(mnt))) i = 1;
Redundant check of mnt != NULL.
> if (mnt->mnt_flags & MNT_NOSUID) i |= 2;
> if (!sysv_valid_dev(inode->i_sb->s_dev))
> return -EOVERFLOW;
> @@ -398,7 +398,7 @@ static int report_statvfs64(struct vfsmo
> int j = strlen (p);
>
> if (j > 15) j = 15;
> - if (IS_RDONLY(inode)) i = 1;
> + if (IS_RDONLY(inode) || (mnt && MNT_IS_RDONLY(mnt))) i = 1;
Redundant check of mnt != NULL
> if (mnt->mnt_flags & MNT_NOSUID) i |= 2;
> if (!sysv_valid_dev(inode->i_sb->s_dev))
> return -EOVERFLOW;
> diff -NurpP --minimal
> linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/namei.c
> linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/namei.c
> --- linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/namei.c
> 2005-02-19 06:31:50 +0100
> +++
> linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/namei.c
> 2005-02-19 06:32:05 +0100
> @@ -220,7 +220,7 @@ int permission(struct inode *inode, int
> /*
>* Nobody gets write access to a read-only fs.
>*/
> - if (IS_RDONLY(inode) &&
> + if ((IS_RDONLY(inode) || (nd && MNT_IS_RDONLY(nd->mnt))) &&
> (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
> return -EROFS;
This is very dodgy. What if the user is calling permission without
setting the (currently optional) nameidata hint? Have you audited the
kernel to find out if this is safe?
> diff -NurpP --minimal
> linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/open.c
> linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/open.c
> --- linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/open.c
> 2005-02-19 06:31:43 +0100
> +++
> linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/open.c
> 2005-02-19 06:32:05 +0100
> @@ -239,7 +239,7 @@ static inline long do_sys_truncate(const
> goto dput_and_out;
>
> error = -EROFS;
> - if (IS_RDONLY(inode))
> + if (IS_RDONLY(inode) || MNT_IS_RDONLY(nd.mnt))
> goto dput_and_out;
>
> error = -EPERM;
> @@ -363,7 +363,7 @@ asmlinkage long sys_utime(char __user *
> inode = nd.dentry->d_inode;
>
> error = -EROFS;
> - if (IS_RDONLY(inode))
> + if (IS_RDONLY(inode) || MNT_IS_RDONLY(nd.mnt))
> goto dput_and_out;
>
> /* Don't worry, the checks are done in inode_change_ok() */
> @@ -420,7 +420,7 @@ long do_utimes(char __user * filename, s
> inode = nd.dentry->d_inode;
>
> error = -EROFS;
> - if (IS_RDONLY(inode))
> + if (IS_RDONLY(inode) || MNT_IS_RDONLY(nd.mnt))
> goto dput_and_out;
>
> /* Don't worry, the checks are done in inode_change_ok() */
> @@ -502,7 +502,8 @@ asmlinkage long sys_access(const char __
> if (!res) {
> res = permission(nd.dentry->d_inode, mode, );
> /* SuS v2 requires we report a read only fs too */
> - if(!res && (mode & S_IWOTH) && IS_RDONLY(nd.dentry->d_inode)
> + if(!res && (mode & S_IWOTH)
> +&& (IS_RDONLY(nd.dentry->d_inode) || MNT_IS_RDONLY(nd.mnt))
> && !special_file(nd.dentry->d_inode->i_mode))
> res = -EROFS;
> path_release();
> @@ -608,7 +609,7 @@ asmlinkage long sys_fchmod(unsigned int
> inode = dentry->d_inode;
>
> err = -EROFS;
> - if (IS_RDONLY(inode))
> + if (IS_RDONLY(inode) || (file && MNT_IS_RDONLY(file->f_vfsmnt)))
> goto out_putf;
Redundant check of file != NULL.
> err = -EPERM;
> if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
> @@ -640,7 +641,7 @@ asmlinkage long sys_chmod(const char __u
> inode = nd.dentry->d_inode;
>
> error = -EROFS;
> - if (IS_RDONLY(inode))
> + if (IS_RDONLY(inode) || MNT_IS_RDONLY(nd.mnt))
> goto dput_and_out;
>
> error = -EPERM;
> diff -NurpP --minimal
>
Re: [Patch 6/6] Bind Mount Extensions 0.06
On Tue, Feb 22, 2005 at 09:34:31AM -0500, Trond Myklebust wrote: > ty den 22.02.2005 Klokka 13:13 (+0100) skreiv Herbert Poetzl: > > > > diff -NurpP --minimal > > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/nfs/dir.c > > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/nfs/dir.c > > --- > > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/nfs/dir.c > > 2005-02-13 17:16:55 +0100 > > +++ > > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/nfs/dir.c > > 2005-02-19 06:32:05 +0100 > > @@ -771,7 +771,8 @@ static int is_atomic_open(struct inode * > > if (nd->flags & LOOKUP_DIRECTORY) > > return 0; > > /* Are we trying to write to a read only partition? */ > > - if (IS_RDONLY(dir) && (nd->intent.open.flags & > > (O_CREAT|O_TRUNC|FMODE_WRITE))) > > + if ((IS_RDONLY(dir) || (nd && MNT_IS_RDONLY(nd->mnt))) && > > + (nd->intent.open.flags & (O_CREAT|O_TRUNC|FMODE_WRITE))) > > return 0; > > return 1; > > } > > The check for nd != NULL is redundant. See 5 lines further up... indeed, thanks! > Cheers, > Trond > > -- > Trond Myklebust <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 6/6] Bind Mount Extensions 0.06
ty den 22.02.2005 Klokka 13:13 (+0100) skreiv Herbert Poetzl: > > diff -NurpP --minimal > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/nfs/dir.c > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/nfs/dir.c > --- linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/nfs/dir.c > 2005-02-13 17:16:55 +0100 > +++ > linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/nfs/dir.c >2005-02-19 06:32:05 +0100 > @@ -771,7 +771,8 @@ static int is_atomic_open(struct inode * > if (nd->flags & LOOKUP_DIRECTORY) > return 0; > /* Are we trying to write to a read only partition? */ > - if (IS_RDONLY(dir) && (nd->intent.open.flags & > (O_CREAT|O_TRUNC|FMODE_WRITE))) > + if ((IS_RDONLY(dir) || (nd && MNT_IS_RDONLY(nd->mnt))) && > + (nd->intent.open.flags & (O_CREAT|O_TRUNC|FMODE_WRITE))) > return 0; > return 1; > } The check for nd != NULL is redundant. See 5 lines further up... Cheers, Trond -- Trond Myklebust <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 6/6] Bind Mount Extensions 0.06
ty den 22.02.2005 Klokka 13:13 (+0100) skreiv Herbert Poetzl: diff -NurpP --minimal linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/nfs/dir.c linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/nfs/dir.c --- linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/nfs/dir.c 2005-02-13 17:16:55 +0100 +++ linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/nfs/dir.c 2005-02-19 06:32:05 +0100 @@ -771,7 +771,8 @@ static int is_atomic_open(struct inode * if (nd-flags LOOKUP_DIRECTORY) return 0; /* Are we trying to write to a read only partition? */ - if (IS_RDONLY(dir) (nd-intent.open.flags (O_CREAT|O_TRUNC|FMODE_WRITE))) + if ((IS_RDONLY(dir) || (nd MNT_IS_RDONLY(nd-mnt))) + (nd-intent.open.flags (O_CREAT|O_TRUNC|FMODE_WRITE))) return 0; return 1; } The check for nd != NULL is redundant. See 5 lines further up... Cheers, Trond -- Trond Myklebust [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 6/6] Bind Mount Extensions 0.06
On Tue, Feb 22, 2005 at 09:34:31AM -0500, Trond Myklebust wrote: ty den 22.02.2005 Klokka 13:13 (+0100) skreiv Herbert Poetzl: diff -NurpP --minimal linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/nfs/dir.c linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/nfs/dir.c --- linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/nfs/dir.c 2005-02-13 17:16:55 +0100 +++ linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/nfs/dir.c 2005-02-19 06:32:05 +0100 @@ -771,7 +771,8 @@ static int is_atomic_open(struct inode * if (nd-flags LOOKUP_DIRECTORY) return 0; /* Are we trying to write to a read only partition? */ - if (IS_RDONLY(dir) (nd-intent.open.flags (O_CREAT|O_TRUNC|FMODE_WRITE))) + if ((IS_RDONLY(dir) || (nd MNT_IS_RDONLY(nd-mnt))) + (nd-intent.open.flags (O_CREAT|O_TRUNC|FMODE_WRITE))) return 0; return 1; } The check for nd != NULL is redundant. See 5 lines further up... indeed, thanks! Cheers, Trond -- Trond Myklebust [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch 6/6] Bind Mount Extensions 0.06
ty den 22.02.2005 Klokka 13:13 (+0100) skreiv Herbert Poetzl:
diff -NurpP --minimal
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/arch/sparc64/solaris/fs.c
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/arch/sparc64/solaris/fs.c
---
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/arch/sparc64/solaris/fs.c
2004-12-25 01:54:50 +0100
+++
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/arch/sparc64/solaris/fs.c
2005-02-19 06:32:05 +0100
@@ -362,7 +362,7 @@ static int report_statvfs(struct vfsmoun
int j = strlen (p);
if (j 15) j = 15;
- if (IS_RDONLY(inode)) i = 1;
+ if (IS_RDONLY(inode) || (mnt MNT_IS_RDONLY(mnt))) i = 1;
Redundant check of mnt != NULL.
if (mnt-mnt_flags MNT_NOSUID) i |= 2;
if (!sysv_valid_dev(inode-i_sb-s_dev))
return -EOVERFLOW;
@@ -398,7 +398,7 @@ static int report_statvfs64(struct vfsmo
int j = strlen (p);
if (j 15) j = 15;
- if (IS_RDONLY(inode)) i = 1;
+ if (IS_RDONLY(inode) || (mnt MNT_IS_RDONLY(mnt))) i = 1;
Redundant check of mnt != NULL
if (mnt-mnt_flags MNT_NOSUID) i |= 2;
if (!sysv_valid_dev(inode-i_sb-s_dev))
return -EOVERFLOW;
diff -NurpP --minimal
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/namei.c
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/namei.c
--- linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/namei.c
2005-02-19 06:31:50 +0100
+++
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/namei.c
2005-02-19 06:32:05 +0100
@@ -220,7 +220,7 @@ int permission(struct inode *inode, int
/*
* Nobody gets write access to a read-only fs.
*/
- if (IS_RDONLY(inode)
+ if ((IS_RDONLY(inode) || (nd MNT_IS_RDONLY(nd-mnt)))
(S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
return -EROFS;
This is very dodgy. What if the user is calling permission without
setting the (currently optional) nameidata hint? Have you audited the
kernel to find out if this is safe?
diff -NurpP --minimal
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/open.c
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/open.c
--- linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/open.c
2005-02-19 06:31:43 +0100
+++
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/open.c
2005-02-19 06:32:05 +0100
@@ -239,7 +239,7 @@ static inline long do_sys_truncate(const
goto dput_and_out;
error = -EROFS;
- if (IS_RDONLY(inode))
+ if (IS_RDONLY(inode) || MNT_IS_RDONLY(nd.mnt))
goto dput_and_out;
error = -EPERM;
@@ -363,7 +363,7 @@ asmlinkage long sys_utime(char __user *
inode = nd.dentry-d_inode;
error = -EROFS;
- if (IS_RDONLY(inode))
+ if (IS_RDONLY(inode) || MNT_IS_RDONLY(nd.mnt))
goto dput_and_out;
/* Don't worry, the checks are done in inode_change_ok() */
@@ -420,7 +420,7 @@ long do_utimes(char __user * filename, s
inode = nd.dentry-d_inode;
error = -EROFS;
- if (IS_RDONLY(inode))
+ if (IS_RDONLY(inode) || MNT_IS_RDONLY(nd.mnt))
goto dput_and_out;
/* Don't worry, the checks are done in inode_change_ok() */
@@ -502,7 +502,8 @@ asmlinkage long sys_access(const char __
if (!res) {
res = permission(nd.dentry-d_inode, mode, nd);
/* SuS v2 requires we report a read only fs too */
- if(!res (mode S_IWOTH) IS_RDONLY(nd.dentry-d_inode)
+ if(!res (mode S_IWOTH)
+ (IS_RDONLY(nd.dentry-d_inode) || MNT_IS_RDONLY(nd.mnt))
!special_file(nd.dentry-d_inode-i_mode))
res = -EROFS;
path_release(nd);
@@ -608,7 +609,7 @@ asmlinkage long sys_fchmod(unsigned int
inode = dentry-d_inode;
err = -EROFS;
- if (IS_RDONLY(inode))
+ if (IS_RDONLY(inode) || (file MNT_IS_RDONLY(file-f_vfsmnt)))
goto out_putf;
Redundant check of file != NULL.
err = -EPERM;
if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
@@ -640,7 +641,7 @@ asmlinkage long sys_chmod(const char __u
inode = nd.dentry-d_inode;
error = -EROFS;
- if (IS_RDONLY(inode))
+ if (IS_RDONLY(inode) || MNT_IS_RDONLY(nd.mnt))
goto dput_and_out;
error = -EPERM;
diff -NurpP --minimal
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01/fs/reiserfs/ioctl.c
linux-2.6.11-rc4-bme0.06-bm0.01-at0.01-cc0.01-co0.01-xa0.01-ro0.01/fs/reiserfs/ioctl.c
---
