linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Richard Guy Briggs <rgb@redhat.com>
To: Cong Wang <xiyou.wangcong@gmail.com>
Cc: linux-audit@redhat.com, Paul Moore <pmoore@redhat.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	David Miller <davem@davemloft.net>,
	Johannes Berg <johannes.berg@intel.com>,
	Florian Westphal <fw@strlen.de>,
	Eric Dumazet <edumazet@google.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	netdev <netdev@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	syzkaller <syzkaller@googlegroups.com>
Subject: Re: netlink: GPF in sock_sndtimeo
Date: Fri, 9 Dec 2016 06:01:55 -0500	[thread overview]
Message-ID: <20161209110155.GW22655@madcap2.tricolour.ca> (raw)
In-Reply-To: <CAM_iQpWRe0tDk=wvXONG7hamH3EtE0nJuAOF1kVKJgdpMvz2DA@mail.gmail.com>

On 2016-12-08 22:57, Cong Wang wrote:
> On Thu, Dec 8, 2016 at 10:02 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > I also tried to extend Cong Wang's idea to attempt to proactively respond to a
> > NETLINK_URELEASE on the audit_sock and reset it, but ran into a locking error
> > stack dump using mutex_lock(&audit_cmd_mutex) in the notifier callback.
> > Eliminating the lock since the sock is dead anways eliminates the error.
> >
> > Is it safe?  I'll resubmit if this looks remotely sane.  Meanwhile I'll try to
> > get the test case to compile.
> 
> It doesn't look safe, because 'audit_sock', 'audit_nlk_portid' and 'audit_pid'
> are updated as a whole and race between audit_receive_msg() and
> NETLINK_URELEASE.

This is what I expected and why I originally added the mutex lock in the
callback...  The dumps I got were bare with no wrapper identifying the
process context or specific error, so I'm at a bit of a loss how to
solve this (without thinking more about it) other than instinctively
removing the mutex.

Another approach might be to look at consolidating the three into one
identifier or derive the other two from one, or serialize their access.

> > @@ -1167,10 +1190,14 @@ static void __net_exit audit_net_exit(struct net *net)
> >  {
> >         struct audit_net *aunet = net_generic(net, audit_net_id);
> >         struct sock *sock = aunet->nlsk;
> > +
> > +       mutex_lock(&audit_cmd_mutex);
> >         if (sock == audit_sock) {
> >                 audit_pid = 0;
> > +               audit_nlk_portid = 0;
> >                 audit_sock = NULL;
> >         }
> > +       mutex_unlock(&audit_cmd_mutex);
> 
> If you decide to use NETLINK_URELEASE notifier, the above piece is no
> longer needed, the net_exit path simply releases a refcnt.

Good point.  It would have already killed it off.  So this piece is
arguably too late anyways.

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

  reply	other threads:[~2016-12-09 11:02 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CACT4Y+aG1+91U1PWMTwpE_6vbEuqG7CdLCM1H=3WVJWtz=>
     [not found] ` <CAM_iQpVeLvfYV+1jX1ZKOntZim4roof4=>
2016-11-29 16:48   ` netlink: GPF in sock_sndtimeo Richard Guy Briggs
2016-11-29 23:13     ` Cong Wang
2016-11-30  4:52       ` Richard Guy Briggs
2016-12-09  6:02         ` Richard Guy Briggs
2016-12-09  6:57           ` Cong Wang
2016-12-09 11:01             ` Richard Guy Briggs [this message]
2016-12-10  4:13               ` Cong Wang
2016-12-10  7:40                 ` Cong Wang
2016-12-12 10:07                   ` Dmitry Vyukov
2016-12-13  7:51                   ` Richard Guy Briggs
2016-12-13  8:28                     ` Richard Guy Briggs
2016-12-12 10:02                 ` Richard Guy Briggs
2016-12-12 10:03                   ` [PATCH v2] audit: use proper refcount locking on audit_sock Richard Guy Briggs
2016-12-12 17:10                     ` Paul Moore
2016-12-13  4:49                       ` Richard Guy Briggs
2016-12-12 20:18                     ` Paul Moore
2016-12-13  5:10                       ` Richard Guy Briggs
2016-12-13 15:01                         ` Richard Guy Briggs
2016-12-12 23:58                     ` Cong Wang
2016-12-13 14:55                       ` Richard Guy Briggs
2016-12-13  0:10                   ` netlink: GPF in sock_sndtimeo Cong Wang
2016-12-13 10:52                     ` Richard Guy Briggs
2016-12-14  0:17                       ` Cong Wang
2016-12-14  4:17                         ` Richard Guy Briggs
2016-12-13 15:03                   ` [RFC PATCH v3] audit: use proper refcount locking on audit_sock Richard Guy Briggs
2016-12-13 20:50                     ` Paul Moore
2016-12-14  0:19                     ` Cong Wang
2016-12-14  4:00                       ` Richard Guy Briggs
2016-12-14  5:36                         ` Cong Wang
2016-12-09 10:49           ` netlink: GPF in sock_sndtimeo Dmitry Vyukov
2016-12-09 11:48             ` Richard Guy Briggs
2016-12-09 11:53               ` Dmitry Vyukov
2016-12-09 12:12                 ` Richard Guy Briggs
2016-11-26 15:44 Dmitry Vyukov
2016-11-26 16:17 ` Eric Dumazet
2016-11-27  1:11 ` Cong Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161209110155.GW22655@madcap2.tricolour.ca \
    --to=rgb@redhat.com \
    --cc=davem@davemloft.net \
    --cc=dvyukov@google.com \
    --cc=edumazet@google.com \
    --cc=fw@strlen.de \
    --cc=herbert@gondor.apana.org.au \
    --cc=johannes.berg@intel.com \
    --cc=linux-audit@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pmoore@redhat.com \
    --cc=syzkaller@googlegroups.com \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).