Re: [PATCH net] net/packet: avoid truncating TPACKET_V3 private size

[Willem de Bruijn]

Mark Amirkan via B4 Relay wrote:
> From: Mark Amirkan <markdamirkan@xxxxxxxxx>
> 
> tpacket_req3.tp_sizeof_priv is an unsigned int, and packet_set_ring()
> validates the full value against the block size.  init_prb_bdqc() then
> stores it in the unsigned short blk_sizeof_priv field.
> 
> Commit 2b6867c2ce76 ("net/packet: fix overflow in check for priv area
> size") fixed the validation arithmetic, but an accepted value above
> USHRT_MAX still narrows when it is stored.
> 
> For a 131072-byte block, tp_sizeof_priv=65536 is valid.  The narrowing
> makes offset_to_first_pkt 48 instead of 65584, so packet records can be
> placed in the private area that userspace asked the kernel to preserve.
> 
> blk_sizeof_priv is internal state, so widen it to hold the validated
> UAPI value.
> 
> Fixes: f6fb8f100b80 ("af-packet: TPACKET_V3 flexible buffer implementation.")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: Symbolic
> Signed-off-by: Mark Amirkan <markdamirkan@xxxxxxxxx>

Reviewed-by: Willem de Bruijn <willemb@xxxxxxxxxx>

> ---
>  net/packet/internal.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/packet/internal.h b/net/packet/internal.h
> index b76e645cd7..f5c8cd0eed 100644
> --- a/net/packet/internal.h
> +++ b/net/packet/internal.h
> @@ -21,7 +21,7 @@ struct tpacket_kbdq_core {
>  	unsigned int	hdrlen;
>  	unsigned char	reset_pending_on_curr_blk;
>  	unsigned short	kactive_blk_num;
> -	unsigned short	blk_sizeof_priv;
> +	unsigned int	blk_sizeof_priv;

This could be moved to avoid growing the struct. There is a 4B hole
further down. But it's no big deal for this struct.