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
| ||
|
Date: Tue, 23 Dec 2014 12:37:17 +0200 From: Jouni Malinen <jkmalinen@...il.com> To: Willem de Bruijn <willemb@...gle.com> Cc: netdev@...r.kernel.org, davem@...emloft.net, eric.dumazet@...il.com, dborkman@...hat.com Subject: Re: [PATCH net-next v2] packet: make packet_snd fail on len smaller than l2 header On Wed, Nov 19, 2014 at 8:10 PM, Willem de Bruijn <willemb@...gle.com> wrote: > When sending packets out with PF_PACKET, SOCK_RAW, ensure that the > packet is at least as long as the device's expected link layer header. > This check already exists in tpacket_snd, but not in packet_snd. Was this supposed to be refusing zero-length payload following the header like the implementation does or accept zero-length payload like this commit message seems to imply? Based on the commit message, I'd assume 14 byte buffer on Ethernet netdev should have been accepted. I just noticed that once pulling this commit in into my automated test setup, one of the test cases started failing because of the change here. That test case was trying to transmit a minimum length Ethernet header using raw packet socket. Not that I care too much since I can easily change the test case to use one octet longer data to send and this was not really a real protocol case, but I wanted to confirm what was the expected behavior here since this commit seems to have number of inconsistent statements between the commit message, actual validation step, debug print, and code comment.. > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > @@ -2095,6 +2095,18 @@ static void tpacket_destruct_skb(struct sk_buff *skb) > +static bool ll_header_truncated(const struct net_device *dev, int len) > +{ > + /* net device doesn't like empty head */ I'm not sure how to interpret "empty head". Is that saying that the data following the header should not be empty? Or that header should be at least hard_header_len? > + if (unlikely(len <= dev->hard_header_len)) { That would be rejecting exactly hard_header_len, i.e,., I would have expected < instead of <= here based on the commit message. > + net_warn_ratelimited("%s: packet size is too short (%d < %d)\n", > + current->comm, len, dev->hard_header_len); But that debug print uses < instead of <= which is not consistent with the actual condition (and yes, I realize this was there even before this commit). - Jouni -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists