* Re: [RFC][PATCH] ovl_create_index(): vfs_mkdir() might succeed leaving dentry negative unhashed [not found] <20180512012557.GJ30522@ZenIV.linux.org.uk> @ 2018-05-14 15:04 ` Miklos Szeredi 2018-05-14 15:39 ` Al Viro 0 siblings, 1 reply; 2+ messages in thread From: Miklos Szeredi @ 2018-05-14 15:04 UTC (permalink / raw) To: Al Viro; +Cc: linux-fsdevel, overlayfs On Sat, May 12, 2018 at 3:25 AM, Al Viro <viro@zeniv.linux.org.uk> wrote: > [same story as with the previous two patches] > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > --- > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c > index 8bede0742619..cdd8f8816d2a 100644 > --- a/fs/overlayfs/copy_up.c > +++ b/fs/overlayfs/copy_up.c > @@ -373,6 +373,22 @@ static int ovl_create_index(struct dentry *dentry, struct dentry *origin, > if (err) > goto out; > > + if (unlikely(d_unhashed(temp))) { > + struct dentry *d = lookup_one_len(temp->d_name.name, > + temp->d_parent, > + temp->d_name.len); > + if (IS_ERR(d)) { > + err = PTR_ERR(d); > + goto out; This violates the "If -1 is returned, no directory shall be created" rule. lookup_one_len() does various permission checks. The normal DAC check is not a worry, because of the lock on the parent. But is it guaranteed that MAC allows lookup if it allowed mkdir? Then there's still the theoretical possibility of the lookup failing with ENOMEM, probably not worth worrying about too much (maybe emit a warning). Thanks, Miklos > + } > + dput(temp); > + temp = d; > + if (d_is_negative(temp)) { > + err = -EIO; > + goto out; > + } > + } > + > err = ovl_set_upper_fh(upper, temp); > if (err) > goto out_cleanup; ^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [RFC][PATCH] ovl_create_index(): vfs_mkdir() might succeed leaving dentry negative unhashed 2018-05-14 15:04 ` [RFC][PATCH] ovl_create_index(): vfs_mkdir() might succeed leaving dentry negative unhashed Miklos Szeredi @ 2018-05-14 15:39 ` Al Viro 0 siblings, 0 replies; 2+ messages in thread From: Al Viro @ 2018-05-14 15:39 UTC (permalink / raw) To: Miklos Szeredi; +Cc: linux-fsdevel, overlayfs On Mon, May 14, 2018 at 05:04:44PM +0200, Miklos Szeredi wrote: > On Sat, May 12, 2018 at 3:25 AM, Al Viro <viro@zeniv.linux.org.uk> wrote: > > [same story as with the previous two patches] > > > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > > --- > > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c > > index 8bede0742619..cdd8f8816d2a 100644 > > --- a/fs/overlayfs/copy_up.c > > +++ b/fs/overlayfs/copy_up.c > > @@ -373,6 +373,22 @@ static int ovl_create_index(struct dentry *dentry, struct dentry *origin, > > if (err) > > goto out; > > > > + if (unlikely(d_unhashed(temp))) { > > + struct dentry *d = lookup_one_len(temp->d_name.name, > > + temp->d_parent, > > + temp->d_name.len); > > + if (IS_ERR(d)) { > > + err = PTR_ERR(d); > > + goto out; > > This violates the "If -1 is returned, no directory shall be created" rule. So does NFS, for that matter... If you use weird filesystems as layers, you get corner cases. Note that on NFS we'll probably have to go for that possibility (== argument unhashed negative after success) on mkdir, in case when it's reexported and somebody is playing silly buggers with races. > lookup_one_len() does various permission checks. The normal DAC check > is not a worry, because of the lock on the parent. But is it > guaranteed that MAC allows lookup if it allowed mkdir? *shrug* Or it can be set so that rmdir won't be allowed afterwards, or... There are all kinds of ways to set idiotic LSM that would screw overlayfs. Doctor, it hurts when I do it... But then you know what I think of Linux S&M community in general... Right now you code would fail with -EIO in all those cases, AFAICS. So you might get a stray -EPERM... ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-05-14 15:39 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20180512012557.GJ30522@ZenIV.linux.org.uk>
2018-05-14 15:04 ` [RFC][PATCH] ovl_create_index(): vfs_mkdir() might succeed leaving dentry negative unhashed Miklos Szeredi
2018-05-14 15:39 ` Al Viro
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox