On 2020-08-21 15:15, Paul Moore wrote:

On Wed, Jul 29, 2020 at 3:41 PM Richard Guy Briggs [off-list ref] wrote:

quoted

On 2020-07-05 11:10, Paul Moore wrote:

quoted

On Sat, Jun 27, 2020 at 9:22 AM Richard Guy Briggs [off-list ref] wrote:

...

quoted

quoted

quoted

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.

I think an "op=" field is the wrong way to link audit container ID to
a particular record.  It may be convenient, but I fear that it would
be overloading the field too much.

Ok, after looking at the field dictionary how about item, rel, ref or rec?
Item perhaps could be added to the OBJ_PID records, but that might be
overloading a field that is already used in PATH records.  "rel" for
relates-to, "ref" for reference to, "rec" for record...  Perhaps pid=
would be enough to tie this record to the OBJ_PID record or the SYSCALL
record, but in the case of network events, the pid may refer to a kernel
thread.

Like I said above, I think it would be good to see an audit event
example including the ptrace/signal information.  This way we can talk
about it on-list and hash out the various solutions if it proves to be
a problem.

See the list posting from 2020-09-29 "auditing signals" pointing to
ghat81 test case about testing ptrace and signals from 18 months ago.

I think I have a way to generate a signal to multiple targets in one
syscall...  The added challenge is to also give those targets different
audit container identifiers.

quoted

quoted

quoted

@@ -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.

I don't think we need to grab an additional reference here, that is
why I asked the question.  The code above grabs a reference for the
audit container ID object associated with the current task and then
drops it before returning; if the current task, and it's associated
audit container ID object, disappears in the middle of the function
we've got much bigger worries :)

I misunderstood your question previously thinking you wanted yet another
reference taken in this case, when in fact it was the opposite and you
thought the one taken here was superfluous.

I don't *need* to grab the additional references here, but those are the
accessor functions that exist, so I either create sub-accessor functions
without the refcount manipulations that called from the primary accessor
functions or open code a reduncancy...  The locking has been updated to
protect the _put by a spin-lock.  Now that I look at it, the 4th to 7th
lines could be bypassed by a cont == NULL check.

It is somewhat hidden now since this sequence of 7 commands has been
abstracted into another function that is called from a second location.

paul moore

- RGB

--
Richard Guy Briggs [off-list ref]
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635