Re: [PATCH ghak90 V9 05/13] audit: log container info of syscalls

On 2020-07-05 11:10, Paul Moore wrote: > On Sat, Jun 27, 2020 at 9:22 AM Richard Guy Briggs <rgb(a)redhat.com&gt; wrote:

> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > > index f03d3eb0752c..9e79645e5c0e 100644 > > --- a/kernel/auditsc.c > > +++ b/kernel/auditsc.c > > @@ -1458,6 +1466,7 @@ static void audit_log_exit(void) > > struct audit_buffer *ab; > > struct audit_aux_data *aux; > > struct audit_names *n; > > + struct audit_contobj *cont; > > > > context->personality = current->personality; > > > > @@ -1541,7 +1550,7 @@ static void audit_log_exit(void) > > for (aux = context->aux_pids; aux; aux = aux->next) { > > struct audit_aux_data_pids *axs = (void *)aux; > > > > - for (i = 0; i < axs->pid_count; i++) > > + for (i = 0; i < axs->pid_count; i++) { > > if (audit_log_pid_context(context, axs->target_pid[i], > > axs->target_auid[i], > > axs->target_uid[i], > > @@ -1549,14 +1558,20 @@ static void audit_log_exit(void) > > axs->target_sid[i], > > axs->target_comm[i])) > > call_panic = 1; > > + audit_log_container_id(context, axs->target_cid[i]); > > + } > > It might be nice to see an audit event example including the > ptrace/signal information. I'm concerned there may be some confusion > about associating the different audit container IDs with the correct > information in the event. This is the subject of ghat81, which is a test for ptrace and signal records. This was the reason I had advocated for an op= field since there is a possibility of multiple contid records per event.

> > @@ -1575,6 +1590,14 @@ static void audit_log_exit(void) > > > > audit_log_proctitle(); > > > > + rcu_read_lock(); > > + cont = _audit_contobj_get(current); > > + rcu_read_unlock(); > > + audit_log_container_id(context, cont); > > + rcu_read_lock(); > > + _audit_contobj_put(cont); > > + rcu_read_unlock(); > > Do we need to grab an additional reference for the audit container > object here? We don't create any additional references here that > persist beyond the lifetime of this function, right? Why do we need another reference? There's one for each pointer pointing to it and so far we have just one from this task. Or are you thinking of the contid hash list, which is only added to when a task points to it and gets removed from that list when the last task stops pointing to it. Later that gets more complicated with network namespaces and nested container objects. For now we just needed it while generating the record, then it gets freed.