On Wed, Feb 5, 2020 at 6:51 PM Richard Guy Briggs [off-list ref] wrote:

On 2020-02-05 18:05, Paul Moore wrote:

quoted

On Thu, Jan 30, 2020 at 2:28 PM Richard Guy Briggs [off-list ref] wrote:

quoted

On 2020-01-22 16:29, Paul Moore wrote:

quoted

On Tue, Dec 31, 2019 at 2:51 PM Richard Guy Briggs [off-list ref] wrote:

quoted

Track the parent container of a container to be able to filter and
report nesting.

Now that we have a way to track and check the parent container of a
container, modify the contid field format to be able to report that
nesting using a carrat ("^") separator to indicate nesting.  The
original field format was "contid=<contid>" for task-associated records
and "contid=<contid>[,<contid>[...]]" for network-namespace-associated
records.  The new field format is
"contid=<contid>[^<contid>[...]][,<contid>[...]]".

Let's make sure we always use a comma as a separator, even when
recording the parent information, for example:
"contid=<contid>[,^<contid>[...]][,<contid>[...]]"

The intent here is to clearly indicate and separate nesting from
parallel use of several containers by one netns.  If we do away with
that distinction, then we lose that inheritance accountability and
should really run the list through a "uniq" function to remove the
produced redundancies.  This clear inheritance is something Steve was
looking for since tracking down individual events/records to show that
inheritance was not aways feasible due to rolled logs or search effort.

Perhaps my example wasn't clear.  I'm not opposed to the little
carat/hat character indicating a container's parent, I just think it
would be good to also include a comma *in*addition* to the carat/hat.

Ah, ok.  Well, I'd offer that it would be slightly shorter, slightly
less cluttered and having already written the parser in userspace, I
think the parser would be slightly simpler.

I must admit, I was a bit puzzled by your snippet of code that was used
as a prefix to the next item rather than as a postfix to the given item.

Can you say why you prefer the comma in addition?

Generally speaking, I believe that a single delimiter is both easier
for the eyes to parse, and easier/safer for machines to parse as well.
In this particular case I think of the comma as a delimiter and the
carat as a modifier, reusing the carat as a delimiter seems like a bad
idea to me.

quoted

quoted

quoted

quoted

diff --git a/kernel/audit.c b/kernel/audit.c
index ef8e07524c46..68be59d1a89b 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c

quoted

@@ -492,6 +493,7 @@ void audit_switch_task_namespaces(struct nsproxy *ns, struct task_struct *p)
                audit_netns_contid_add(new->net_ns, contid);
 }

+void audit_log_contid(struct audit_buffer *ab, u64 contid);

If we need a forward declaration, might as well just move it up near
the top of the file with the rest of the declarations.

Ok.

quoted

quoted

+void audit_log_contid(struct audit_buffer *ab, u64 contid)
+{
+       struct audit_contobj *cont = NULL, *prcont = NULL;
+       int h;

It seems safer to pass the audit container ID object and not the u64.

It would also be faster, but in some places it isn't available such as
for ptrace and signal targets.  This also links back to the drop record
refcounts to hold onto the contobj until process exit, or signal
delivery.

What we could do is to supply two potential parameters, a contobj and/or
a contid, and have it use the contobj if it is valid, otherwise, use the
contid, as is done for names and paths supplied to audit_log_name().

Let's not do multiple parameters, that begs for misuse, let's take the
wrapper function route:

 func a(int id) {
   // important stuff
 }

 func ao(struct obj) {
   a(obj.id);
 }

... and we can add a comment that you *really* should be using the
variant that passes an object.

I was already doing that where it available, and dereferencing the id
for the call.  But I see an advantage to having both parameters supplied
to the function, since it saves us the trouble of dereferencing it,
searching for the id in the hash list and re-locating the object if the
object is already available.

I strongly prefer we not do multiple parameters for the same "thing";
I would much rather do the wrapper approach as described above.  I
would also like to see us use the audit container ID object as much as
possible, using a bare integer should be a last resort.

-- 
paul moore
www.paul-moore.com