Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1541524

Re: [RFC PATCH v3] audit: use proper refcount locking on audit_sock

Path csiph.com!eternal-september.org!feeder.eternal-september.org!news.unit0.net!news.panservice.it!bofh.it!news.nic.it!robomod
From Paul Moore <paul@paul-moore.com>
Newsgroups linux.kernel
Subject Re: [RFC PATCH v3] audit: use proper refcount locking on audit_sock
Date Tue, 13 Dec 2016 22:00:02 +0100
Message-ID <sO2l4-5Po-31@gated-at.bofh.it> (permalink)
References <sNvIu-30q-41@gated-at.bofh.it> <sNWSm-2IE-49@gated-at.bofh.it>
Dkim-Signature v=1; a=rsa-sha256; c=relaxed/relaxed; d=paul-moore-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=X7DTDKc23Y/Y+4usxf0EVrGcyhqCtA4LkcbZXTlcqTY=; b=qE27F+JMoEIFEtk1mO9UYmvrNDN74dCizzoG83MVSuEsi6wiJP2eJIt2StR/8+Ujq3 fRE4c6qgmSvLZXn3IEfP6ai1qVdIXBUnhBFm46ACzsu8qmnAktTP1RgH1sc2/Lj93k7y SIZSG7TqC5xxfDM3wZL0jpFKZCzCCyROmAzyN3IUQw/gj1lJIrTn+4nASo+SeoWJa4QL HUo8wpyLWxupisaTHJZ11r7XYmjjA7GtuRp5svvF62LfAZfWvc7UipA79aMYcoBjIcVA 6L5U9nLMN5u6P+JEkahYO51WqbN8CBks7ScB+yWD4L1DIqAE9NmknX/pkCTP2vn3z3NQ Cgrw==
X-Google-Dkim-Signature v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=X7DTDKc23Y/Y+4usxf0EVrGcyhqCtA4LkcbZXTlcqTY=; b=DIa8l2+YqXvnm1FwhG0bp1a1fNHZNZc/oqIpHFH76LlK8lTw7F0pQY8mguj+Vf/bse SXJ+cyGfXCc6YSlV/0bnk7uBi5ZRNZt7vB4EyQpKu3W+2gbP6umenk/f+lG7qxRWNVON RDJm8jh1Bh1Wh24VOfBu/0S3gUCfkwPJXTSTSnWw4vnoSTv6YwZ+Rfi646enWkUZpVJ4 YyVpmUe1BILoP+MeFtORQ7WTS9SFL7VmpO7riARtJdNU8EznnpsXVHXEf5bqba0Cdgj8 /Wl0zWVOO6SxaV9hvNlHbdG3hAXGMpR5x1jl3BkJLVKM9AGGQVncwUDyhaZsC6Fd3KIF bMzA==
X-Gm-Message-State AKaTC0192tcBBz5sR1bROH8jzS+hfunUsZi1jnolsi4FCGA5Ey4oz4AQLVfV9dfCg5xOqXjic1AFWlWF1r2hCw==
X-Received by 10.31.166.12 with SMTP id p12mr35637314vke.50.1481662218970; Tue, 13 Dec 2016 12:50:18 -0800 (PST)
MIME-Version 1.0
X-Originating-IP [96.230.190.88]
Content-Type text/plain; charset=UTF-8
Sender robomod@news.nic.it
List-ID <linux-kernel.vger.kernel.org>
X-Mailing-List linux-kernel@vger.kernel.org
Approved robomod@news.nic.it
Lines 47
Organization linux.* mail to news gateway
X-Original-Cc netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-audit@redhat.com, edumazet@google.com, xiyou.wangcong@gmail.com, dvyukov@google.com
X-Original-Date Tue, 13 Dec 2016 15:50:18 -0500
X-Original-Message-ID <CAHC9VhSpMKS+9mDRiS2ekpJWCKsVSZGgJV-3v_T2+9D7PNGUnA@mail.gmail.com>
X-Original-References <20161212100215.GA1305@madcap2.tricolour.ca> <61c37ca790bc11bc023aea8f9b70ab3098aa30f5.1481626466.git.rgb@redhat.com>
X-Original-Sender linux-kernel-owner@vger.kernel.org
Xref csiph.com linux.kernel:1541524

Show key headers only | View raw


On Tue, Dec 13, 2016 at 10:03 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> Resetting audit_sock appears to be racy.
>
> audit_sock was being copied and dereferenced without using a refcount on
> the source sock.
>
> Bump the refcount on the underlying sock when we store a refrence in
> audit_sock and release it when we reset audit_sock.  audit_sock
> modification needs the audit_cmd_mutex.
>
> See: https://lkml.org/lkml/2016/11/26/232
>
> Thanks to Eric Dumazet <edumazet@google.com> and Cong Wang
> <xiyou.wangcong@gmail.com> on ideas how to fix it.
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> There has been a lot of change in the audit code that is about to go
> upstream to address audit queue issues.  This patch is based on the
> source tree: git://git.infradead.org/users/pcmoore/audit#next
> ---
>  kernel/audit.c |   28 +++++++++++++++++++++++-----
>  1 files changed, 23 insertions(+), 5 deletions(-)

This looks more reasonable.  I still wonder about synchronization
between threads changing the audit_* connection variables and the
kauditd_thread, but I guess we can treat that as another issue; this
patch fixes a bug and is worth merging now.

I'm building a test kernel right now, assuming nothing blows up I'll
push this patch with the rest of the audit patches tomorrow; if
something bad happens, this is going to miss the first audit pull
request.

> diff --git a/kernel/audit.c b/kernel/audit.c
> index f20eee0..3bb4126 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -446,14 +446,19 @@ static void kauditd_retry_skb(struct sk_buff *skb)
>   * Description:
>   * Break the auditd/kauditd connection and move all the records in the retry
>   * queue into the hold queue in case auditd reconnects.
> + * The audit_cmd_mutex must be held when calling this function.
>   */

Don't resend, but in the future please start comments like this on the
previous line.

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: netlink: GPF in sock_sndtimeo Richard Guy Briggs <rgb@redhat.com> - 2016-12-12 11:10 +0100
  Re: netlink: GPF in sock_sndtimeo Cong Wang <xiyou.wangcong@gmail.com> - 2016-12-13 01:20 +0100
    Re: netlink: GPF in sock_sndtimeo Richard Guy Briggs <rgb@redhat.com> - 2016-12-13 12:00 +0100
      Re: netlink: GPF in sock_sndtimeo Cong Wang <xiyou.wangcong@gmail.com> - 2016-12-14 01:40 +0100
        Re: netlink: GPF in sock_sndtimeo Richard Guy Briggs <rgb@redhat.com> - 2016-12-14 05:20 +0100
  [RFC PATCH v3] audit: use proper refcount locking on audit_sock Richard Guy Briggs <rgb@redhat.com> - 2016-12-13 16:10 +0100
    Re: [RFC PATCH v3] audit: use proper refcount locking on audit_sock Paul Moore <paul@paul-moore.com> - 2016-12-13 22:00 +0100
    Re: [RFC PATCH v3] audit: use proper refcount locking on audit_sock Cong Wang <xiyou.wangcong@gmail.com> - 2016-12-14 01:20 +0100
      Re: [RFC PATCH v3] audit: use proper refcount locking on audit_sock Richard Guy Briggs <rgb@redhat.com> - 2016-12-14 05:10 +0100
        Re: [RFC PATCH v3] audit: use proper refcount locking on audit_sock Cong Wang <xiyou.wangcong@gmail.com> - 2016-12-14 07:40 +0100

csiph-web