| Message ID | 20250410171725.1265860-1-mic@digikod.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/3] landlock: Log the TGID of the domain creator | expand |
On Thu, Apr 10, 2025 at 07:17:21PM +0200, Mickaël Salaün wrote: > As for other Audit's "pid" fields, Landlock should use the task's TGID > instead of its TID. Fix this issue by keeping a reference to the TGID > of the domain creator. > > Existing tests already check for the PID but only with the thread group > leader, so always the TGID. A following patch adds dedicated tests for > non-leader thread. > > Remove the current_real_cred() check which does not make sense because > we only reference a struct pid, whereas a previous version did reference > a struct cred instead. > > Cc: Christian Brauner <brauner@kernel.org> > Cc: Günther Noack <gnoack@google.com> > Cc: Paul Moore <paul@paul-moore.com> > Signed-off-by: Mickaël Salaün <mic@digikod.net> > --- > security/landlock/domain.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/security/landlock/domain.c b/security/landlock/domain.c > index bae2e9909013..a647b68e8d06 100644 > --- a/security/landlock/domain.c > +++ b/security/landlock/domain.c > @@ -16,6 +16,7 @@ > #include <linux/path.h> > #include <linux/pid.h> > #include <linux/sched.h> > +#include <linux/signal.h> > #include <linux/uidgid.h> > > #include "access.h" > @@ -99,8 +100,7 @@ static struct landlock_details *get_current_details(void) > return ERR_PTR(-ENOMEM); > > memcpy(details->exe_path, path_str, path_size); > - WARN_ON_ONCE(current_cred() != current_real_cred()); > - details->pid = get_pid(task_pid(current)); > + details->pid = get_pid(task_tgid(current)); > details->uid = from_kuid(&init_user_ns, current_uid()); > get_task_comm(details->comm, current); > return details; > -- > 2.49.0 > Ah, a classic! Good catch finding this early enough for 6.15! Reviewed-by: Günther Noack <gnoack3000@gmail.com>
diff --git a/security/landlock/domain.c b/security/landlock/domain.c index bae2e9909013..a647b68e8d06 100644 --- a/security/landlock/domain.c +++ b/security/landlock/domain.c @@ -16,6 +16,7 @@ #include <linux/path.h> #include <linux/pid.h> #include <linux/sched.h> +#include <linux/signal.h> #include <linux/uidgid.h> #include "access.h" @@ -99,8 +100,7 @@ static struct landlock_details *get_current_details(void) return ERR_PTR(-ENOMEM); memcpy(details->exe_path, path_str, path_size); - WARN_ON_ONCE(current_cred() != current_real_cred()); - details->pid = get_pid(task_pid(current)); + details->pid = get_pid(task_tgid(current)); details->uid = from_kuid(&init_user_ns, current_uid()); get_task_comm(details->comm, current); return details;
As for other Audit's "pid" fields, Landlock should use the task's TGID instead of its TID. Fix this issue by keeping a reference to the TGID of the domain creator. Existing tests already check for the PID but only with the thread group leader, so always the TGID. A following patch adds dedicated tests for non-leader thread. Remove the current_real_cred() check which does not make sense because we only reference a struct pid, whereas a previous version did reference a struct cred instead. Cc: Christian Brauner <brauner@kernel.org> Cc: Günther Noack <gnoack@google.com> Cc: Paul Moore <paul@paul-moore.com> Signed-off-by: Mickaël Salaün <mic@digikod.net> --- security/landlock/domain.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)