Re: [PATCH 09/26] make access() use mnt check
On Sat, 2007-06-30 at 10:37 +0100, Christoph Hellwig wrote:
> > --- lxc/fs/namei.c~numa_mnt_want_write 2007-06-25 11:05:50.0 -0700
> > +++ lxc-dave/fs/namei.c 2007-06-25 11:05:50.0 -0700
> > @@ -230,10 +230,12 @@ int permission(struct inode *inode, int
> > int retval, submask;
> >
> > if (mask & MAY_WRITE) {
> > -
> > /*
> > -* Nobody gets write access to a read-only fs.
> > +* If this WARN_ON() is hit, it likely means that
> > +* there was a missed mnt_want_write() on the path
> > +* leading here.
> > */
> > + WARN_ON(__mnt_is_readonly(nd->mnt));
> > if (IS_RDONLY(inode) &&
>
> I might be missing something, but wouldn't this trip an
>
> access("/foo/bar", W_OK);
>
> On a readonly filesystem?
Yes, it will. I will re-work it a bit.
-- Dave
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/26] make access() use mnt check
On Mon, Jun 25, 2007 at 11:27:25AM -0700, Dave Hansen wrote:
> I've got this in the next set:
>
> -
> - if(IS_RDONLY(nd.dentry->d_inode))
> + /*
> +* This is a rare case where using __mnt_is_readonly()
> +* is OK without a mnt_want/drop_write() pair. Since
> +* not actual write to the fs is performed here, we do
> +* not need to telegraph to that to anyone. Also, we
> +* accept that access is inherently racy, and know that
> +* the fs might be remounted between this syscall, and
> +* any action taken because of its result.
> +*/
> + if (__mnt_is_readonly(nd.mnt))
> res = -EROFS;
Looks good.
> diff -puN fs/namei.c~numa_mnt_want_write fs/namei.c
> --- lxc/fs/namei.c~numa_mnt_want_write 2007-06-25 11:05:50.0 -0700
> +++ lxc-dave/fs/namei.c 2007-06-25 11:05:50.0 -0700
> @@ -230,10 +230,12 @@ int permission(struct inode *inode, int
> int retval, submask;
>
> if (mask & MAY_WRITE) {
> -
> /*
> -* Nobody gets write access to a read-only fs.
> +* If this WARN_ON() is hit, it likely means that
> +* there was a missed mnt_want_write() on the path
> +* leading here.
> */
> + WARN_ON(__mnt_is_readonly(nd->mnt));
> if (IS_RDONLY(inode) &&
I might be missing something, but wouldn't this trip an
access("/foo/bar", W_OK);
On a readonly filesystem?
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/26] make access() use mnt check
On Mon, 2007-06-25 at 11:27 -0700, Dave Hansen wrote: > On Sat, 2007-06-23 at 08:45 +0100, Christoph Hellwig wrote: > > You probably want to add a big comment explaining why it's fine here. > > I've got this in the next set: > > - > - if(IS_RDONLY(nd.dentry->d_inode)) > + /* > +* This is a rare case where using __mnt_is_readonly() > +* is OK without a mnt_want/drop_write() pair. Since > +* not actual write to the fs is performed here, we do s/not/no/ > +* not need to telegraph to that to anyone. Also, we > +* accept that access is inherently racy, and know that > +* the fs might be remounted between this syscall, and > +* any action taken because of its result. > +*/ > + if (__mnt_is_readonly(nd.mnt)) > res = -EROFS; > -- David Kleikamp IBM Linux Technology Center - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/26] make access() use mnt check
On Sat, 2007-06-23 at 08:45 +0100, Christoph Hellwig wrote:
> On Fri, Jun 22, 2007 at 01:03:14PM -0700, Dave Hansen wrote:
> >
> > It is OK to let access() go without using a mnt_want/drop_write()
> > pair because it doesn't actually do writes to the filesystem,
> > and it is inherently racy anyway. This is a rare case when it is
> > OK to use __mnt_is_readonly() directly.
>
> You probably want to add a big comment explaining why it's fine here.
I've got this in the next set:
-
- if(IS_RDONLY(nd.dentry->d_inode))
+ /*
+* This is a rare case where using __mnt_is_readonly()
+* is OK without a mnt_want/drop_write() pair. Since
+* not actual write to the fs is performed here, we do
+* not need to telegraph to that to anyone. Also, we
+* accept that access is inherently racy, and know that
+* the fs might be remounted between this syscall, and
+* any action taken because of its result.
+*/
+ if (__mnt_is_readonly(nd.mnt))
res = -EROFS;
> That reminds me of something else I had in mind to debug that the
> writer counts are okay:
>
> we should probably add a check in permission that we have an elevated
> writercount on the vfsmount/sb. Of course we'll need some way to
> overrid it for access(), which means passing down a flag to it or
> something.
This was already in the second to last patch in the series. Good enough?
diff -puN fs/namei.c~numa_mnt_want_write fs/namei.c
--- lxc/fs/namei.c~numa_mnt_want_write 2007-06-25 11:05:50.0 -0700
+++ lxc-dave/fs/namei.c 2007-06-25 11:05:50.0 -0700
@@ -230,10 +230,12 @@ int permission(struct inode *inode, int
int retval, submask;
if (mask & MAY_WRITE) {
-
/*
-* Nobody gets write access to a read-only fs.
+* If this WARN_ON() is hit, it likely means that
+* there was a missed mnt_want_write() on the path
+* leading here.
*/
+ WARN_ON(__mnt_is_readonly(nd->mnt));
if (IS_RDONLY(inode) &&
-- Dave
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/26] make access() use mnt check
On Fri, Jun 22, 2007 at 01:03:14PM -0700, Dave Hansen wrote: > > It is OK to let access() go without using a mnt_want/drop_write() > pair because it doesn't actually do writes to the filesystem, > and it is inherently racy anyway. This is a rare case when it is > OK to use __mnt_is_readonly() directly. You probably want to add a big comment explaining why it's fine here. That reminds me of something else I had in mind to debug that the writer counts are okay: we should probably add a check in permission that we have an elevated writercount on the vfsmount/sb. Of course we'll need some way to overrid it for access(), which means passing down a flag to it or something. - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/26] make access() use mnt check
It is OK to let access() go without using a mnt_want/drop_write() pair because it doesn't actually do writes to the filesystem, and it is inherently racy anyway. This is a rare case when it is OK to use __mnt_is_readonly() directly. Signed-off-by: Dave Hansen <[EMAIL PROTECTED]> --- lxc-dave/fs/open.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff -puN fs/open.c~make-access-use-helper fs/open.c --- lxc/fs/open.c~make-access-use-helper2007-06-21 23:23:17.0 -0700 +++ lxc-dave/fs/open.c 2007-06-21 23:23:17.0 -0700 @@ -395,7 +395,7 @@ asmlinkage long sys_faccessat(int dfd, c special_file(nd.dentry->d_inode->i_mode)) goto out_path_release; - if(IS_RDONLY(nd.dentry->d_inode)) + if (__mnt_is_readonly(nd.mnt)) res = -EROFS; out_path_release: _ - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
