netdev - Re: [Patch net] net: saving irq context for peernet2id()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 20 Oct 2016 16:35:53 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Paul Moore <paul@...l-moore.com>
Cc:     Stephen Smalley <sds@...ho.nsa.gov>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Elad Raz <e@...draz.com>, Richard Guy Briggs <rgb@...hat.com>
Subject: Re: [Patch net] net: saving irq context for peernet2id()

On Thu, Oct 20, 2016 at 12:07 PM, Paul Moore <paul@...l-moore.com> wrote:
> On Thu, Oct 20, 2016 at 2:29 PM, Cong Wang <xiyou.wangcong@...il.com> wrote:
>> On Thu, Oct 20, 2016 at 7:58 AM, Stephen Smalley <sds@...ho.nsa.gov> wrote:
>>> On 10/20/2016 02:52 AM, Cong Wang wrote:
>>>> A kernel warning inside __local_bh_enable_ip() was reported by people
>>>> running SELinux, this is caused due to some SELinux functions
>>>> (indirectly) call peernet2id() with IRQ disabled in process context,
>>>> when we re-enable BH with IRQ disabled kernel complains. Shut up this
>>>> warning by saving IRQ context in peernet2id(), BH is still implicitly
>>>> disabled.
>>>
>>> Not sure this suffices; kill_fasync() -> send_sigio() ->
>>> send_sigio_to_task() -> sigio_perm() -> security_file_send_sigiotask()
>>> -> selinux_file_send_sigiotask() -> ... -> audit_log() -> ... ->
>>> peernet2id()
>>
>> Oh, this is a new one. kill_fasync() is called in IRQ handler, so we actually
>> do multicast in IRQ context.... It makes no sense, netlink multicast could
>> be very expensive if we have many listeners.
>
> I'm sure there are a few others I don't know about, but I believe the
> only commonly used audit multicast listener is systemd.

But user-space is free to listen to this group, right? If so this is just open
for a potential DDOS attack.

>
>> I am Cc'ing Richard who added that multicast in audit_log_end(). It seems
>> not easy to just move the multicast to a workqueue, since the skb is copied
>> from audit_buffer which is freed immediately after that, probably need another
>> queue like audit_skb_queue.
>
> This approach would double the queue size which is something I want to
> avoid.  I would suggest sticking with a single queue and dealing with
> the netlink message link fixup and multicast send in the existing
> netlink unicast thread; basically we would just be moving the
> multicast code from audit_log_end() into kauditd_thread().  This is
> the same approach I mentioned earlier off-list.

This is what I did in the follow up patch. I attach the updated version
in this email for you to review, I still can't make selinux-testsuites working
on my Fedora even though I have SELinux=enforcing, anyhow I don't
see any kernel warning in my dmesg at least.

>
> However, that isn't something I want to mess with as a regression fix,
> mostly because I really want to see this regression gone by -rc2 as it
> is making SELinux testing a real pain.  If the patch posted at the top
> of this thread isn't a suitable fix, we really should revert the
> original patch.

Since you want to test SELinux anyway, please test the attached one.

Thanks.

View attachment "audit-v2.diff" of type "text/plain" (2202 bytes)