From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753451AbcLMKwk (ORCPT ); Tue, 13 Dec 2016 05:52:40 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56628 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752746AbcLMKwi (ORCPT ); Tue, 13 Dec 2016 05:52:38 -0500 Date: Tue, 13 Dec 2016 05:52:33 -0500 From: Richard Guy Briggs To: Cong Wang Cc: linux-audit@redhat.com, Paul Moore , Dmitry Vyukov , David Miller , Johannes Berg , Florian Westphal , Eric Dumazet , Herbert Xu , netdev , LKML , syzkaller Subject: Re: netlink: GPF in sock_sndtimeo Message-ID: <20161213105233.GG1305@madcap2.tricolour.ca> References: <20161129164859.GD26673@madcap2.tricolour.ca> <20161130045207.GE26673@madcap2.tricolour.ca> <20161209060248.GT22655@madcap2.tricolour.ca> <20161209110155.GW22655@madcap2.tricolour.ca> <20161212100215.GA1305@madcap2.tricolour.ca> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Tue, 13 Dec 2016 10:52:38 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016-12-12 16:10, Cong Wang wrote: > On Mon, Dec 12, 2016 at 2:02 AM, Richard Guy Briggs wrote: > > On 2016-12-09 20:13, Cong Wang wrote: > >> Netlink notifier can safely be converted to blocking one, I will send > >> a patch. > > > > I had a quick look at how that might happen. The netlink notifier chain > > is atomic. Would the registered callback funciton need to spawn a > > one-time thread to avoid blocking? > > It is already non-atomic now: > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=efa172f42836477bf1ac3c9a3053140df764699c Ok, that is recent... It is still less attractive as you point out due to the overhead, but still worth considering if we can't find another way. > > I had a look at your patch. It looks attractively simple. The audit > > next tree has patches queued that add an audit_reset function that will > > require more work. I still see some potential gaps. > > > > - If the process messes up (or the sock lookup messes up) it is reset > > in the kauditd thread under the audit_cmd_mutex. > > > > - If the process exits normally or is replaced due to an audit_replace > > error, it is reset from audit_receive_skb under the audit_cmd_mutex. > > > > - If the process dies before the kauditd thread notices, either reap it > > via notifier callback or it needs a check on net exit to reset. This > > last one appears necessary to decrement the sock refcount so the sock > > can be released in netlink_kernel_release(). > > > > If we want to be proactive and use the netlink notifier, we assume the > > overhead of adding to the netlink notifier chain and eliminate all the > > other reset calls under the kauditd thread. If we are ok being > > reactionary, then we'll at least need the net exit check on audit_sock. > > I don't see why we need to check it in net exit if we use refcnt, > because we have two different users of audit_sock: kauditd and > netns, if both take care of refcnt properly, we don't need to worry > about who is the last, no matter what failures occur in what order. It is actually the audit_pid and audit_nlk_portid that I care about more. The audit daemon could vanish or close the socket while the kernel sock to which it was attached is still quite valid. Accessing the set of three atomically is the urge. I wonder if it makes more sense to test for the presence of auditd using audit_sock rather than audit_pid, but still keep audit_pid for our reporting and replacement strategy. Another idea would be to put the three in one struct. Can someone explain how they think the original test was able to trigger this GPF? Network namespace shutdown while something pretended to set up a new auditd? That's impressive for a fuzzer if that's the case... Is there an strace? I guess it is all in test(). - RGB -- Richard Guy Briggs Kernel Security Engineering, Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635