Tejun Heo <tj@xxxxxxxxxx> writes:
> On Tue, Aug 11, 2015 at 07:58:14PM -0500, Eric W. Biederman wrote:
>> Andy Lutomirski <luto@xxxxxxxxxxxxxx> writes:
>>
>> > On Tue, Aug 11, 2015 at 11:57 AM, Eric W. Biederman
>> > <ebiederm@xxxxxxxxxxxx> 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.
proc has always reported -ENOENT. sysfs is the odd one out.
I am not completely certain that trivial patch above, does not introduce
a leak, a NULL pointer dereference or something else nasty when the code
is hit.
So far it seems that no one cares. And since the change is brittle I am
not inclined to mess with it this week, as I have other demands on my
limited review bandwidth right now.
Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
