Received: by 2002:a25:8b91:0:0:0:0:0 with SMTP id j17csp5266438ybl; Wed, 22 Jan 2020 13:32:25 -0800 (PST) X-Google-Smtp-Source: APXvYqxilxdgpqN4BxOKGqq9L0iVRsok+nDbwts5MJxONv3sUYXT98/jmujazky0a1C1pxDyboCq X-Received: by 2002:a9d:7d81:: with SMTP id j1mr9056083otn.267.1579728745768; Wed, 22 Jan 2020 13:32:25 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1579728745; cv=none; d=google.com; s=arc-20160816; b=ydF9N1tGlXmaofk8Ry1halIyLrCKrnScYqHTv1kogDDstmrKh0Rv6eLOOJ7kO0zMVm tA6YM01H1PQz6iB+8WCCof18pdwdoYCvjPM1YPny+JqKBbdVmD7yyc7ylKw9lN6xUPFC T11C/zeISfXyMKnsPhUoQFYVP2JVLpKmwlVf+Jd5DS+Ca1oBTlmlg+X17dcmpejauI2S kS6RkrSRTpIOBGVG6BxEeMnCA9UQZyOxZcbbD+H65nETQkKTX3Vrk2hyxLwL/SQGKnzh TsCluZZriFngPzozEN5BOhGnh/iizKG4JcePxnSQRXNv6w+67jAPYuUozjsd+0NuhNEV lYjg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=f0IkEaXGVt8hUwF5uD44/2Lbilo5B8J7tUK5oD4D1tM=; b=wiy5tEJyHKHkNGiiZs3sr4jb8XXoGTMgQZFoTuwqA/SnPHmnhI4c9HWMobQ7wNXTdN 14Utze+qY51mxkPR1euth2LoBentxfBi5ULUqo4gVhSgDndkpn5fAzREPxzEG4kcK1/+ B6tKX9jM9DLhM/DdWwt4VpXmD3hAemYzvRRfJ9zbFUX2dDJfK7wLcm0I4pvYs8TnRAC3 SZLTihWFw1KPSofgBZPVw55sQAmRNEUvLr2NyA5XuZ76B9wJ26KigwDEbzEvK0I5WUTR pVXjIeWuBcv0letYN5Hs53O4dnwbqRYWB6TU5Vq9Ow5qn4vtAkvezA4PtlFZWik/hnHP ZaqA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@paul-moore-com.20150623.gappssmtp.com header.s=20150623 header.b=IMKvzt5h; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id f60si5604otf.119.2020.01.22.13.32.13; Wed, 22 Jan 2020 13:32:25 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@paul-moore-com.20150623.gappssmtp.com header.s=20150623 header.b=IMKvzt5h; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729412AbgAVV31 (ORCPT + 99 others); Wed, 22 Jan 2020 16:29:27 -0500 Received: from mail-lf1-f65.google.com ([209.85.167.65]:35691 "EHLO mail-lf1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729393AbgAVV30 (ORCPT ); Wed, 22 Jan 2020 16:29:26 -0500 Received: by mail-lf1-f65.google.com with SMTP id z18so744409lfe.2 for ; Wed, 22 Jan 2020 13:29:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=paul-moore-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=f0IkEaXGVt8hUwF5uD44/2Lbilo5B8J7tUK5oD4D1tM=; b=IMKvzt5hA/l/NbMQKPL+I7Jyo3nfkoDyZBLi5RRpV+j/tCfVTPajPZF/8GU6ionRxK +mQvxp5xyCs/lbcj6fFzQN4Qx1QZwbZnFyqVJt1eXrhrtdv8R8zb6thx4q6gtOe7qdbv 5E93iQ92ILpO7fxqk7N35pyU4vQtv7jxP2GP0fuciK3ZgZYXrrSxeC+vto4ftrmoX6xE BMLerP1Bzr4rimUoCrUubcW9bUpU+oy6tgpr2A1RG4zmzYvb7Ds2n6dNPujUpm52og8K 3daDlG3WlViN5WCY+t1+i4y8O4eenhXIrO7AMnatPzf7Iv6Jx0Do09fsis7ZX1cIrVTs 3HLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=f0IkEaXGVt8hUwF5uD44/2Lbilo5B8J7tUK5oD4D1tM=; b=p+JMLu5tr8p/d8sPQ67ik8g1Wbv16twaUgqSV2GaEANRjtaVx1p0IuOWpHFdAmWZFF esxW42xRW6uPi2QvyRcw3tG2cEP1ZS5CQdzKsev2/6HsOO/myjFModRIhSu8nGCGPHM8 n0XvujKZ++D1QHB/5DjE0/+P6WSHZxtZ0LfEm5ZaozbMj+wXoY2pVNc48RDLjp4mCIIO hW15erNDBoG/6mOdoSbsZVV1DTQfpd3uArkBfvIdYVi1T/yZSSsU7JFt258vUBqC6vMs NRZY/S+UBdLoKDcZXmBTZrnYkQ6Gbs1ahrfSJZNFfeErnXog467ANizT25LZnd/TkTrd xTGg== X-Gm-Message-State: APjAAAWHFql6lm/npU3RLcJMoH3DgOULUinRKBpgcuhNLfUnjg8Z1hhZ hU+7WwWo53vqHWHXr7bdQVVTZpNepBeA29NEPKUA X-Received: by 2002:ac2:5f59:: with SMTP id 25mr2754662lfz.193.1579728564136; Wed, 22 Jan 2020 13:29:24 -0800 (PST) MIME-Version: 1.0 References: <6452955c1e038227a5cd169f689f3fd3db27513f.1577736799.git.rgb@redhat.com> In-Reply-To: <6452955c1e038227a5cd169f689f3fd3db27513f.1577736799.git.rgb@redhat.com> From: Paul Moore Date: Wed, 22 Jan 2020 16:29:12 -0500 Message-ID: Subject: Re: [PATCH ghak90 V8 13/16] audit: track container nesting To: Richard Guy Briggs Cc: containers@lists.linux-foundation.org, linux-api@vger.kernel.org, Linux-Audit Mailing List , linux-fsdevel@vger.kernel.org, LKML , netdev@vger.kernel.org, netfilter-devel@vger.kernel.org, sgrubb@redhat.com, omosnace@redhat.com, dhowells@redhat.com, simo@redhat.com, Eric Paris , Serge Hallyn , ebiederm@xmission.com, nhorman@tuxdriver.com, Dan Walsh , mpatel@redhat.com Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Dec 31, 2019 at 2:51 PM Richard Guy Briggs wrote: > > 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=" for task-associated records > and "contid=[,[...]]" for network-namespace-associated > records. The new field format is > "contid=[^[...]][,[...]]". Let's make sure we always use a comma as a separator, even when recording the parent information, for example: "contid=[,^[...]][,[...]]" > Signed-off-by: Richard Guy Briggs > --- > include/linux/audit.h | 1 + > kernel/audit.c | 53 +++++++++++++++++++++++++++++++++++++++++++-------- > kernel/audit.h | 1 + > kernel/auditfilter.c | 17 ++++++++++++++++- > kernel/auditsc.c | 2 +- > 5 files changed, 64 insertions(+), 10 deletions(-) ... > diff --git a/kernel/audit.c b/kernel/audit.c > index ef8e07524c46..68be59d1a89b 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -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. > +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. > + if (!audit_contid_valid(contid)) { > + audit_log_format(ab, "%llu", contid); Do we really want to print (u64)-1 here? Since this is a known invalid number, would "?" be a better choice? > + return; > + } > + h = audit_hash_contid(contid); > + rcu_read_lock(); > + list_for_each_entry_rcu(cont, &audit_contid_hash[h], list) > + if (cont->id == contid) { > + prcont = cont; Why not just pull the code below into the body of this if statement? It all needs to be done under the RCU read lock anyway and the code would read much better this way. > + break; > + } > + if (!prcont) { > + audit_log_format(ab, "%llu", contid); > + goto out; > + } > + while (prcont) { > + audit_log_format(ab, "%llu", prcont->id); > + prcont = prcont->parent; > + if (prcont) > + audit_log_format(ab, "^"); In the interest of limiting the number of calls to audit_log_format(), how about something like the following: audit_log_format("%llu", cont); iter = cont->parent; while (iter) { if (iter->parent) audit_log_format("^%llu,", iter); else audit_log_format("^%llu", iter); iter = iter->parent; } > + } > +out: > + rcu_read_unlock(); > +} > + > /* > * audit_log_container_id - report container info > * @context: task or local context for record ... > @@ -2705,9 +2741,10 @@ int audit_set_contid(struct task_struct *task, u64 contid) > if (!ab) > return rc; > > - audit_log_format(ab, > - "op=set opid=%d contid=%llu old-contid=%llu", > - task_tgid_nr(task), contid, oldcontid); > + audit_log_format(ab, "op=set opid=%d contid=", task_tgid_nr(task)); > + audit_log_contid(ab, contid); > + audit_log_format(ab, " old-contid="); > + audit_log_contid(ab, oldcontid); This is an interesting case where contid and old-contid are going to be largely the same, only the first (current) ID is going to be different; do we want to duplicate all of those IDs? > audit_log_end(ab); > return rc; > } > @@ -2723,9 +2760,9 @@ void audit_log_container_drop(void) -- paul moore www.paul-moore.com