Re: [PATCH 09/26] make access() use mnt check

2007-07-02 Thread Dave Hansen 
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

2007-06-30 Thread Christoph Hellwig 
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

2007-06-26 Thread Dave Kleikamp 
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

2007-06-23 Thread Christoph Hellwig 
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

2007-06-22 Thread Dave Hansen 

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