Re: [PATCH] audit: add CLONE3 auxiliary record to log process cloning

[Steve Grubb]

On Tuesday, July 28, 2026 12:31:02 PM Eastern Daylight Time Richard Guy 
Briggs wrote:
> On 2026-07-24 12:02, Ricardo Robaina wrote:
> > The clone3(2) syscall moves most parameters to struct clone_args.
> > For this reason, the generic SYSCALL audit record does not capture
> > the structured arguments.
> > 
> > Add a CLONE3 auxiliary record that logs: flags, exit_signal, cgroup,
> > and pidfd fields from struct clone_args. When CLONE_PIDFD is set and
> > the syscall succeeds, the resolved pidfd is logged; otherwise
> > pidfd=(null).
> > 
> >  ----
> >  type=SYSCALL : syscall=clone3 a0=0x7ffe7f1ec640 a1=0x58 a2=0x0 ...
> >  type=CLONE3 : cl3_flags=0x1000 exit_signal=17 cgroup=0 pidfd=3
> > 
> > Link: https://github.com/linux-audit/audit-kernel/issues/151
> > Signed-off-by: Ricardo Robaina <rrobaina@xxxxxxxxxx>
> > ---
> > 
> >  include/linux/audit.h      | 10 ++++++++++
> >  include/uapi/linux/audit.h |  1 +
> >  kernel/auditsc.c           | 20 ++++++++++++++++++++
> >  kernel/fork.c              |  4 +++-
> >  4 files changed, 34 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 45abb3722d30..833f349bf415 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -449,6 +449,7 @@ extern void __audit_tk_injoffset(struct timespec64
> > offset);> 
> >  extern void __audit_ntp_log(const struct audit_ntp_data *ad);
> >  extern void __audit_log_nfcfg(const char *name, u8 af, unsigned int
> >  nentries,>  
> >  			      enum audit_nfcfgop op, gfp_t gfp);
> > 
> > +extern void __audit_log_clone3(struct kernel_clone_args *kargs, int
> > ret);
> > 
> >  static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
> >  {
> > 
> > @@ -598,6 +599,12 @@ static inline void audit_log_nfcfg(const char *name,
> > u8 af,> 
> >  		__audit_log_nfcfg(name, af, nentries, op, gfp);
> >  
> >  }
> > 
> > +static inline void audit_log_clone3(struct kernel_clone_args *kargs, int
> > ret) +{
> > +	if (!audit_dummy_context())
> > +		__audit_log_clone3(kargs, ret);
> > +}
> > +
> > 
> >  extern int audit_n_rules;
> >  extern int audit_signals;
> >  #else /* CONFIG_AUDITSYSCALL */
> > 
> > @@ -730,6 +737,9 @@ static inline void audit_log_nfcfg(const char *name,
> > u8 af,> 
> >  				   enum audit_nfcfgop op, gfp_t gfp)
> >  
> >  { }
> > 
> > +static inline void audit_log_clone3(struct kernel_clone_args *kargs, int
> > ret) +{ }
> > +
> > 
> >  #define audit_n_rules 0
> >  #define audit_signals 0
> >  #endif /* CONFIG_AUDITSYSCALL */
> > 
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index e8f5ce677df7..37357e17adbf 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -122,6 +122,7 @@
> > 
> >  #define AUDIT_OPENAT2		1337	/* Record showing openat2 how args 
*/
> >  #define AUDIT_DM_CTRL		1338	/* Device Mapper target control */
> >  #define AUDIT_DM_EVENT		1339	/* Device Mapper events */
> > 
> > +#define AUDIT_CLONE3		1343	/* Record showing clone3 args */
> > 
> >  #define AUDIT_AVC		1400	/* SE Linux avc denial or grant */
> >  #define AUDIT_SELINUX_ERR	1401	/* Internal SE Linux Errors */
> > 
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 6610e667c728..c0106bb71c19 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -2882,6 +2882,26 @@ void __audit_log_nfcfg(const char *name, u8 af,
> > unsigned int nentries,> 
> >  }
> >  EXPORT_SYMBOL_GPL(__audit_log_nfcfg);
> > 
> > +void __audit_log_clone3(struct kernel_clone_args *kargs, int ret)
> > +{
> > +	struct audit_buffer *ab;
> > +	int pidfd;
> > +
> > +	ab = audit_log_start(audit_context(), GFP_KERNEL,
> > +			     AUDIT_CLONE3);
> > +	if (!ab)
> > +		return;
> > +
> > +	audit_log_format(ab, "cl3_flags=0x%llx exit_signal=%d cgroup=%d",
> > +			 kargs->flags, kargs->exit_signal, kargs->cgroup);
> 
> Would there be any confusion if this were simply "flags=..." since it is
> already a record type AUDIT_CLONE3?

flags is already used in mmap records and will require workarounds to have 
the same field name here. Separating them by name is better.

> > +	if ((kargs->flags & CLONE_PIDFD) && ret >= 0 &&
> > +	    !get_user(pidfd, kargs->pidfd))
> > +		audit_log_format(ab, " pidfd=%d", pidfd);
> > +	else
> > +		audit_log_format(ab, " pidfd=(null)");
> 
> Is there any interest in:
> 	child_tid (was captured by generic audit syscall arg 4)
> 	parent_tid (was captured by generic audit syscall arg 3 or 4)
> 	tls (was captured by generic audit syscall arg 4)
> 	set_tid

Nope. Those are more resource related and not security focused. Even if you 
wanted it, these are pointers to the answer and not the answer. The patch has 
everything needed from a security PoV.

Ack.

> Is there a situation where a valid pid is returned without CLONE_PIDFD set?

No idea. The man page makes it sound like it is only valid when CLONE_PIDFD. 
Besides, its a pointer to an int from what the man page said.

-Steve

> Otherwise, looks reasonable to me.
> 
> > +	audit_log_end(ab);
> > +}
> > +
> > 
> >  static void audit_log_task(struct audit_buffer *ab)
> >  {
> >  
> >  	kuid_t auid, uid;
> > 
> > diff --git a/kernel/fork.c b/kernel/fork.c
> > index f0e2e131a9a5..0f52e8c0e900 100644
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -3047,7 +3047,9 @@ SYSCALL_DEFINE2(clone3, struct clone_args __user *,
> > uargs, size_t, size)> 
> >  	if (!clone3_args_valid(&kargs))
> >  	
> >  		return -EINVAL;
> > 
> > -	return kernel_clone(&kargs);
> > +	err = kernel_clone(&kargs);
> > +	audit_log_clone3(&kargs, err);
> > +	return err;
> > 
> >  }
> >  
> >  void walk_process_tree(struct task_struct *top, proc_visitor visitor,
> >  void *data)
> - RGB
> 
> --
> Richard Guy Briggs <rgb@xxxxxxxxxx>
> Sr. S/W Engineer, Kernel Security, Base Operating Systems
> Remote, Ottawa, Red Hat Canada
> Upstream IRC: SunRaycer
> Voice: +1.613.860 2354 SMS: +1.613.518.6570