linux-kernel - Re: [PATCH v2 1/2] procfs: avoid some usages of seq

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: <69d70bb9-4c9c-4a7d-a5bf-e5af8c98ba0b@yandex.ru>
Date: Fri, 8 Nov 2024 19:03:30 +0300
From: stsp <stsp2@...dex.ru>
To: Kees Cook <kees@...nel.org>, linux-kernel@...r.kernel.org
Cc: Eric Biederman <ebiederm@...ssion.com>, Andy Lutomirski
 <luto@...nel.org>, Aleksa Sarai <cyphar@...har.com>,
 Alexander Viro <viro@...iv.linux.org.uk>,
 Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
 Thomas Gleixner <tglx@...utronix.de>, Jeff Layton <jlayton@...nel.org>,
 John Johansen <john.johansen@...onical.com>,
 Chengming Zhou <chengming.zhou@...ux.dev>,
 Casey Schaufler <casey@...aufler-ca.com>,
 Adrian Ratiu <adrian.ratiu@...labora.com>,
 Felix Moessbauer <felix.moessbauer@...mens.com>, Jens Axboe
 <axboe@...nel.dk>, Oleg Nesterov <oleg@...hat.com>,
 "Serge E. Hallyn" <serge@...lyn.com>, linux-fsdevel@...r.kernel.org,
 "Peter Zijlstra (Intel)" <peterz@...radead.org>
Subject: Re: [PATCH v2 1/2] procfs: avoid some usages of seq_file private data

08.11.2024 18:32, Kees Cook пишет:
>
> On November 8, 2024 2:13:38 AM PST, Stas Sergeev <stsp2@...dex.ru> wrote:
>> seq_file private data carries the inode pointer here.
>> Replace
>> `struct inode *inode = m->private;`
>> with:
>> `struct inode *inode = file_inode(m->file);`
>> to avoid the reliance on private data.
> Conceptually this seems good, though I'd expect to see the removal of _setting_ m->private too in this patch.

Sure I can try to do that, perhaps as
an unrelated patch.
Just got scared to post large patches
and deal with potential problems where
I didn't even mean to change anything.

>> This is needed so that `proc_single_show()` can be used by
>> custom fops that utilize seq_file private data for other things.
>> This is used in the next patch.
> Now that next patch is pretty wild. I think using proc is totally wrong for managing uid/gid. If that's going to happen at all, I think it should be tied to pidfd which will already do the correct process lifetime management, etc.
I did the POC with pidfd:

https://lore.kernel.org/lkml/20241101202657.468595-1-stsp2@yandex.ru/T/

For me it was just a random place to
hack a POC on. I then searched for something
more realistic and choose proc/status because
it already carries all the needed info inside.
So in this case it can be read from, validated,
then applied with ioctl.

How exactly do you foresee using pidfd?
I mean, unless I am misunderstanding the
pidfd intention, it is always opened by the
pid of another process. There is no way of
some process to "allow" others opening its
pid, or to even know they did. Is it possible
to use pidfd in such a way that the process
can grant his groups explicitly?
In my POC I had to do a totally silly thing
of opening the _own_ pid and then sending
the resulting fd with SCM_RIGHTS. I don't
think people would do something like that.
What technique do you have in the mind?