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]

Message-ID: <CAKgT0Ud-QEtTTdzGzKkxnextMu63RZL9f2Hp=4VGxiUbeMcUkQ@mail.gmail.com>
Date:	Thu, 16 Jun 2016 11:58:33 -0700
From:	Alexander Duyck <alexander.duyck@...il.com>
To:	Tom Herbert <tom@...bertland.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 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.

- Alex