netdev - Re: [PATCH net-next 1/8] net: Change SKB_GSO_DODGY to be a tx

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 for Android: free password hash cracker in your pocket

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

Date:	Thu, 16 Jun 2016 13:18:03 -0700
From:	Tom Herbert <tom@...bertland.com>
To:	Alexander Duyck <alexander.duyck@...il.com>
Cc:	David Miller <davem@...emloft.net>,
	Netdev <netdev@...r.kernel.org>, Kernel Team <kernel-team@...com>
Subject: Re: [PATCH net-next 1/8] net: Change SKB_GSO_DODGY to be a tx_flag

On Thu, Jun 16, 2016 at 11:58 AM, Alexander Duyck
<alexander.duyck@...il.com> wrote:
> On Thu, Jun 16, 2016 at 10:51 AM, Tom Herbert <tom@...bertland.com> wrote:
>> This replaces gso_type SKB_GSO_DODGY with a new tx_flag named
>> SKBTX_UNTRUSTED_SOURCE. This more generically desrcibes the skb
>> being created from a untrusted source as a characteristic of and skbuff.
>> This also frees up one gso_type flag bit.
>>
>> Signed-off-by: Tom Herbert <tom@...bertland.com>
>
> Instead of leaving this bit in the shared_info why not look at moving
> it into the sk_buff itself?  It seems like this might be a better
> candidate for something like that as a large part of what the dodgy
> bit represents is that the header offsets are likely not set correctly
> and need to be parsed out and updated.  It might make more sense to
> place this in the slot just after remcsum_offload.  That way once all
> the header offsets have been updated you could just update this one
> bit to indicate that the header offsets stored in this sk_buff are
> valid.
>
> I also don't see where these changes address any changes needed to
> skb_gso_ok in order to actually trigger the partial walk though the
> GSO code.  You probably need to look at adding a statement there to do
> a check for your untrusted source bit versus the GSO_ROBUST feature.
> It probably doesn't need to be much, just something like tacking on a
> "&& (!skb_is_untrustued(skb) || (features & NETIF_F_GSO_ROBUST)" to
> the conditional statement.
>
All the places where SKB_GSO_DODGY was being checked should have been
replaced with SKBTX_UNTRUSTED_SOURCE.

> - Alex