lkml.org 
[lkml]   [2007]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC][PATCH 2/7] get mount write in __dentry_open()
    From
    Date
    On Thu, 2007-10-11 at 20:31 +0200, Miklos Szeredi wrote:
    > > On Thu, 2007-10-11 at 17:08 +0200, Miklos Szeredi wrote:
    > > > > diff -puN fs/namei.c~get-write-in-__dentry_open fs/namei.c
    > > > > --- lxc/fs/namei.c~get-write-in-__dentry_open 2007-10-03 14:44:52.000000000 -0700
    > > > > +++ lxc-dave/fs/namei.c 2007-10-04 18:02:48.000000000 -0700
    > > > > @@ -1621,14 +1621,6 @@ int may_open(struct nameidata *nd, int a
    > > > > return -EACCES;
    > > > >
    > > > > flag &= ~O_TRUNC;
    > > > > - } else if (flag & FMODE_WRITE) {
    > > > > - /*
    > > > > - * effectively: !special_file()
    > > > > - * balanced by __fput()
    > > > > - */
    > > > > - error = mnt_want_write(nd->mnt);
    > > > > - if (error)
    > > > > - return error;
    > > > > }
    > > >
    > > > Maybe readonly should still be checked here, so that the order of
    > > > error checking doesn't change. If racing with a read-only remount the
    > > > order is irrelevant anyway. Something like this?
    > > >
    > > > } else if (flag & FMODE_WRITE && __mnt_is_readonly(nd->mnt)) {
    > > > return -EROFS
    > > > }
    > >
    > > I think that would be a bug if anything actually managed to trip that
    > > code. all of the may_open() calls should have been covered by the
    > > __dentry_open() mnt writer.
    >
    > AFACIS, __dentry_open() will normally be called later than may_open().
    > And we don't want it earlier, because ->open() may have side affects,
    > that could be unsafe if done before permission checking.

    I actually check the mount write count before the ->open() in
    __dentry_open(). The truncates are also definitely wrapped in their own
    mnt_want_write() calls now.

    > > > And they should be added around do_truncate() as well, since you
    > > > remove the protection from may_open().
    > > >
    > > > This one introduces an interesting race between ro-remount and
    > > > open(O_TRUNC), where the truncate can succeed but the open fail with
    > > > EROFS. Is that a problem?
    > >
    > > You're right, this does introduce that race, and it is relatively hard
    > > to fix properly. But, the 'return a filp' patch makes it easy to fix.
    > > I've put a temporary kludge in the updated version of this patch, and
    > > fixed it properly in that later patch.
    >
    > If you fix this properly, that should take care of the first problem
    > as well.

    Yup. New series coming up.

    -- Dave

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-10-11 21:27    [W:0.034 / U:30.564 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site