> 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.
> > 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.
Miklos
-
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/
