linux-kernel - Re: [GIT PULL] tracing: A few more fixes for 6.7

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=wiwQtUHvzwyZucDq8=Gtw+AnwScyLhpFswrQ84PjhoGsg@mail.gmail.com>
Date: Thu, 21 Dec 2023 09:45:43 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Masami Hiramatsu <mhiramat@...nel.org>, 
	Mark Rutland <mark.rutland@....com>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, 
	Randy Dunlap <rdunlap@...radead.org>, Alexander Graf <graf@...zon.com>
Subject: Re: [GIT PULL] tracing: A few more fixes for 6.7

On Thu, 21 Dec 2023 at 07:26, Steven Rostedt <rostedt@...dmis.org> wrote:
>
> - Fix eventfs files to inherit the ownership of its parent directory.
>   The dynamic creating of dentries in eventfs did not take into
>   account if the tracefs file system was mounted with a gid/uid,
>   and would still default to the gid/uid of root. This is a regression.

Honestly, this seems to still be entirely buggy. In fact, it looks
buggy in two different ways:

 (a) if 'attr' is NULL, none of this logic is triggered, and uid/gid
is still left as root despite the explicit mount options

 (b) if somebody has done a chown/gid on the directory, the new
dynamic creation logic seems to create any files inside that directory
with the new uid/gid.

Maybe (a) cannot happen, but that code in update_inode_attr() does
have a check for a NULL attr, so either it can happen, or that check
is bogus.

And (b) just looks messy.  Maybe you've disallowed chown/chgid on
tracefs, I didn't check. But why would it inherit the parent uid/gid?
That just doesn't seem to make any sense at all.

I still claim that the whole dynamic ftrace stuff was a huge mistake,
and that the real solution should always have been to just use one
single inode for every file (and use that 'attr' that you track and
the '->getattr()' callback to make them all *look* different to
users).

               Linus