netdev - Re: Sending short raw packets using sendmsg() broke

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] [day] [month] [year] [list]

Message-ID: <CAF=yD-Lg8_Y3KhXGCQWX1tAQtHKqGKik2bHAon+Czyb-RX53xA@mail.gmail.com>
Date:	Fri, 4 Mar 2016 15:52:04 -0500
From:	Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:	Alan Cox <alan@...ux.intel.com>
Cc:	David Miller <davem@...emloft.net>,
	Heikki Hannikainen <hessu@....iki.fi>,
	Network Development <netdev@...r.kernel.org>,
	Willem de Bruijn <willemb@...gle.com>
Subject: Re: Sending short raw packets using sendmsg() broke

On Fri, Mar 4, 2016 at 11:33 AM, Willem de Bruijn
<willemdebruijn.kernel@...il.com> wrote:
> On Fri, Mar 4, 2016 at 10:54 AM, Alan Cox <alan@...ux.intel.com> wrote:
>>> > A quick search for ethhdr in drivers/net/ethernet shows, for
>>> > instance,
>>> > bnx2x_select_queue casting skb->data to an ethernet header. Reading
>>> > nonsense in that particular function is quite safe and given the
>>> > skbuff layout (skb_shared_info) code will never read beyond an
>>> > allocated region. But that was just the first occurrence I found.
>>> > efx_tso_check_protocol is another example.
>>
>> So would always allocating that much space be a good mitigation in
>> general
>
> Agreed. The existing packet allocation path does that by relying on
> LL_RESERVED_SPACE on allocation, not only packet length. I won't
> modify that. But I should perhaps zero up to hard_header_len on
> variable length headers.
>
>>  and perhaps then making the logic check validate() IFF
>> CAP_SYS_RAWIO is not set.
>>
>> A user with CAP_SYS_RAWIO already has the power to control the device
>> by banging registers so the check is not a security loss.
>
> One concern is namespaces. I'll use capable(CAP_SYS_RAWIO), not ns_capable.
>
> Need to add an ax25_validate implemention and run some tests before I
> send out the patch.

Sent a v1. I will try to test the ax25 specific code. Heikki, if you
can, please test it, too.

The change to tpacket_fill_skb will cause a merge conflict with
net-next because of my tpacket gso changes in
http://patchwork.ozlabs.org/patch/578623/

>
> Thanks,
>
>   Willem