linux-kernel - Re: [PATCH] seccomp: Add group_leader pid to seccomp

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: <87h7wc4zac.fsf@x220.int.ebiederm.org>
Date:   Mon, 18 May 2020 18:08:11 -0500
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Sargun Dhillon <sargun@...gun.me>
Cc:     linux-kernel@...r.kernel.org,
        containers@...ts.linux-foundation.org, linux-api@...r.kernel.org,
        christian.brauner@...ntu.com, tycho@...ho.ws,
        keescook@...omium.org, cyphar@...har.com
Subject: Re: [PATCH] seccomp: Add group_leader pid to seccomp_notif

Sargun Dhillon <sargun@...gun.me> writes:

> This includes the thread group leader ID in the seccomp_notif. This is
> immediately useful for opening up a pidfd for the group leader, as
> pidfds only work on group leaders.

The code looks fine (except for the name of the test), but can you
please talk and think about this as something other than the
group leader?

The initial thread in a thread group can die, and the tgid is still
valid for the entire group.  Because the initial thread of a
process/thread group can die (but rarely does) that tends to result in
kernel code that fails when thread_group_leader dies.

To remove that class of bugs I am slowy working to remove the
thread_group_leader from the kernel entirely.

Looking at the names of the fields in the structure it looks like
there is another class of bugs to be removed by renaming PIDTYPE_PID
to PIDTYPE_TID in the kernel as well.  Just skimming the example code
it looks very simple to get confused.

Is there any chance some can modify struct seccomp_notify to do
{
	...
        union {
		__u32 pid;
                __u32 tid;
	};
        ...
}

Just to reduce the chance of confusion between the userspace pid and the
in kernel pid names?

Eric