netdev - Re: [PATCH net] af_packet: fix the tx skb protocol in raw sockets with ETH_P

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, 18 Mar 2019 18:12:29 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     willemdebruijn.kernel@...il.com
Cc:     komachi.yoshiki@....ntt.co.jp, netdev@...r.kernel.org,
        ysk@...sfc.keio.ac.jp, maximmi@...lanox.com
Subject: Re: [PATCH net] af_packet: fix the tx skb protocol in raw sockets
 with ETH_P_ALL

From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Date: Mon, 18 Mar 2019 10:49:56 -0400

> On Mon, Mar 18, 2019 at 1:41 AM Yoshiki Komachi
> <komachi.yoshiki@....ntt.co.jp> wrote:
>>
>> I am using "protocol ip" filters in TC to manipulate TC flower
>> classifiers, which are only available with "protocol ip". However,
>> I faced an issue that packets sent via raw sockets with ETH_P_ALL
>> did not match the ip filters even if they did satisfy the condition
>> (e.g., DHCP offer from dhcpd).
>>
>> I have determined that the behavior was caused by an unexpected
>> value stored in skb->protocol, namely, ETH_P_ALL instead of ETH_P_IP,
>> when packets were sent via raw sockets with ETH_P_ALL set.
>>
>> IMHO, storing ETH_P_ALL in skb->protocol is not appropriate for
>> packets sent via raw sockets because ETH_P_ALL is not a real ether
>> type used on wire, but a virtual one.
>>
>> This patch fixes the tx protocol selection in cases of transmission
>> via raw sockets created with ETH_P_ALL so that it asks the driver to
>> extract protocol from the Ethernet header.
>>
>> Signed-off-by: Yoshiki Komachi <komachi.yoshiki@....ntt.co.jp>
> 
> Acked-by: Willem de Bruijn <willemb@...gle.com>

Applied.

> The Fixes tag is missing. Calling sendmsg on a packet socket bound to
> ETH_P_ALL goes back a long way. It is a user, not kernel, bug to do
> so.
> 
> But no more than sending on a socket bound to ETH_P_NONE (0), which
> was addressed in commit c72219b75fde ("packet: infer protocol from
> ethernet header if unset").
> 
> Probing with packet_parse_headers is available only as of commit
> 75c65772c3 ("net/packet: Ask driver for protocol if not provided by
> user") in v5.1-rc1. This fix does not need to go back further than
> that, imho.

I added 75c65772c3 as the Fixes: tag, and will not queue this up for
-stable.

Thanks for the analysis.