Re: [CFT][PATCH 09/10] sysfs: Create mountpoints with sysfs_create_empty_dir

[Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

On Tue, Aug 11, 2015 at 07:58:14PM -0500, Eric W. Biederman wrote:
> Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> writes:
> 
> > On Tue, Aug 11, 2015 at 11:57 AM, Eric W. Biederman
> > <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote:
> >>
> >> *Boggle*
> >>
> >> The only time this should prevent anything is when in a container when
> >> you are not global root.  And then only mounting sysfs should be
> >> affected.
> >
> > Before:
> >
> > open("/sys/kernel/debug/asdf", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK,
> > 0666) = -1 EACCES (Permission denied)
> >
> >
> > After:
> >
> > open("/sys/kernel/debug/asdf", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK,
> > 0666) = -1 ENOENT (No such file or directory)
> >
> > Something broke.  I don't know whether CentOS cares about that change,
> > but there could be other odd side effects.
> 
> Thanks for pointing this out.  I don't know if broke is quite the right
> word for a change in error codes on lookup failure, but I agree it is a
> difference that could have affected something.
> 
> The behavior of empty proc dirs actually return -ENOENT in this
> situation and so it is a little fuzzy about which is the best behavior
> to use.
> 
> Creativing a negative dentry and and then letting vfs_create fail may be
> the better way to go.
> 
> Negative dentries are weird enough that I would prefer not to have code
> that creates negative dentries.  They could easily be a lurking trap
> for some filesystems dentry operations.
> 
> The patch below is enough to change the error code if someone who can
> reproduce this wants to try this.
> 
> Eric
> 
> diff --gdiff --git a/fs/libfs.c b/fs/libfs.c
> index 102edfd39000..3a452a485cbf 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -1109,7 +1109,7 @@ EXPORT_SYMBOL(simple_symlink_inode_operations);
>   */
>  static struct dentry *empty_dir_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags)
>  {
> -       return ERR_PTR(-ENOENT);
> +       return NULL;

And let's please restore this too.  Sentiments about negative dentries
aside, It's outright wrong to report -ENOENT on creat.

Thanks.

-- 
tejun