REGRESSION?: debugfs: inode: debugfs_create_dir uses mode permission from parent

REGRESSION?: debugfs: inode: debugfs_create_dir uses mode permission from parent

[John Stultz]

Hey all,
  I noticed recently that linus/master (plus patches) stopped booting
to UI on HiKey960, and I bisected the issue down to:
92170b62f1c1 ("debugfs: inode: debugfs_create_dir uses mode permission
from parent")

On the HiKey960 board, we mount debugfs via:
   mount debugfs /sys/kernel/debug /sys/kernel/debug mode=755

But since the change, it seems most of the nodes in /sys/kernel/debug
are: drwx------

Which ends up breaking the egl library, keeping it from loading.

This seems to be the expected behavior of the patch, but I'm surprised
the mode override is now ignored. It seems like the potential fix here
would be to set the /sys/kernel/debug dir to 755 prior to mounting? Or
is there some other advised solution?

thanks
-john

Re: REGRESSION?: debugfs: inode: debugfs_create_dir uses mode permission from parent

[Kees Cook]

On Tue, Jun 12, 2018 at 3:40 PM, John Stultz <john.stultz@linaro.org> wrote:
> Hey all,
>   I noticed recently that linus/master (plus patches) stopped booting
> to UI on HiKey960, and I bisected the issue down to:
> 92170b62f1c1 ("debugfs: inode: debugfs_create_dir uses mode permission
> from parent")
>
> On the HiKey960 board, we mount debugfs via:
>    mount debugfs /sys/kernel/debug /sys/kernel/debug mode=755
>
> But since the change, it seems most of the nodes in /sys/kernel/debug
> are: drwx------
>
> Which ends up breaking the egl library, keeping it from loading.
>
> This seems to be the expected behavior of the patch, but I'm surprised
> the mode override is now ignored. It seems like the potential fix here
> would be to set the /sys/kernel/debug dir to 755 prior to mounting? Or
> is there some other advised solution?

This change has already be reverted from other people reporting the
problem. Perhaps the root cause is the lack of propagation of the
mode= value?

-Kees

-- 
Kees Cook
Pixel Security

Re: REGRESSION?: debugfs: inode: debugfs_create_dir uses mode permission from parent

[John Stultz]

On Tue, Jun 12, 2018 at 3:52 PM, Kees Cook <keescook@chromium.org> wrote:
> On Tue, Jun 12, 2018 at 3:40 PM, John Stultz <john.stultz@linaro.org> wrote:
>> Hey all,
>>   I noticed recently that linus/master (plus patches) stopped booting
>> to UI on HiKey960, and I bisected the issue down to:
>> 92170b62f1c1 ("debugfs: inode: debugfs_create_dir uses mode permission
>> from parent")
>>
>> On the HiKey960 board, we mount debugfs via:
>>    mount debugfs /sys/kernel/debug /sys/kernel/debug mode=755
>>
>> But since the change, it seems most of the nodes in /sys/kernel/debug
>> are: drwx------
>>
>> Which ends up breaking the egl library, keeping it from loading.
>>
>> This seems to be the expected behavior of the patch, but I'm surprised
>> the mode override is now ignored. It seems like the potential fix here
>> would be to set the /sys/kernel/debug dir to 755 prior to mounting? Or
>> is there some other advised solution?
>
> This change has already be reverted from other people reporting the
> problem. Perhaps the root cause is the lack of propagation of the
> mode= value?

Ah! Thanks, sorry for not searching for the earlier discussion.

thanks
-john

Re: REGRESSION?: debugfs: inode: debugfs_create_dir uses mode permission from parent

[Greg Kroah-Hartman]

On Tue, Jun 12, 2018 at 03:40:41PM -0700, John Stultz wrote:
> Hey all,
>   I noticed recently that linus/master (plus patches) stopped booting
> to UI on HiKey960, and I bisected the issue down to:
> 92170b62f1c1 ("debugfs: inode: debugfs_create_dir uses mode permission
> from parent")
> 
> On the HiKey960 board, we mount debugfs via:
>    mount debugfs /sys/kernel/debug /sys/kernel/debug mode=755
> 
> But since the change, it seems most of the nodes in /sys/kernel/debug
> are: drwx------
> 
> Which ends up breaking the egl library, keeping it from loading.

While the debugfs change is now reverted in Linus's tree, I find it
"odd" that a debugfs change would cause egl from loading entirely.  No
userspace code should depend on debugfs files being there or not.  If it
does, that's a bug as debugfs is for _debugging_ stuff, it should not be
an interface between user/kernel that userspace requires for basic
functionality like booting.

And yes, I know all about the crazy qualcom batter api, where they
shoved it into debugfs and ignored the built-in kernel api, that should
be fixed now.  If this is a new one-of-those types of problem, I need to
know so it can get resolved.

thanks,

greg k-h