netdev - Re: [PATCH] net: move padding in struct skb_shared

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:   Mon, 10 Apr 2017 07:43:26 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Alexey Dobriyan <adobriyan@...il.com>
Cc:     davem@...emloft.net, steffen.klassert@...unet.com,
        edumazet@...gle.com, netdev@...r.kernel.org
Subject: Re: [PATCH] net: move padding in struct skb_shared_info

On Mon, 2017-04-10 at 11:07 +0300, Alexey Dobriyan wrote:
> commit 7f564528a480084e2318cd48caba7aef4a54a77f
> ("skbuff: Extend gso_type to unsigned int.") created padding as first
> field of struct skb_shared_info requiring [R64+imm8] addressing mode
> for all fields.
> 
> Patch bubbles up padding brinding code size down to original levels and
> even smaller:
> 
> 	add/remove: 0/0 grow/shrink: 4/304 up/down: 20/-720 (-700)
> 	function                                     old     new   delta
> 	iwl_trans_pcie_tx                           3994    4006     +12
> 	tap_do_read                                 1070    1074      +4
> 	packet_recvmsg                              1155    1157      +2
> 	be_xmit                                     2038    2040      +2
> 	zerocopy_sg_from_iter                        455     454      -1
> 		...
> 	__ef4_rx_packet                             1358    1349      -9
> 	hix5hd2_poll                                1787    1777     -10
> 	e1000_clean_jumbo_rx_irq                    3599    3587     -12
> 	skb_try_coalesce                            1118    1105     -13
> 	xenvif_tx_build_gops                        5057    5043     -14
> 
> Signed-off-by: Alexey Dobriyan <adobriyan@...il.com>
> ---
> 
>  include/linux/skbuff.h |    1 -
>  1 file changed, 1 deletion(-)
> 
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -413,7 +413,6 @@ struct ubuf_info {
>   * the end of the header data, ie. at skb->end.
>   */
>  struct skb_shared_info {
> -	unsigned short	_unused;
>  	unsigned char	nr_frags;
>  	__u8		tx_flags;
>  	unsigned short	gso_size;

Nack

This exact placement was discussed at Netconf and Netdev.

We had off-by-one errors in the past leading to nr_frags being mangled,
and some exploits were quite happy to use these bugs.

Some shuffling in shared_info might help us to find other bugs, and give
more work to security researchers