| Message ID | 1438081021-1321-1-git-send-email-al.drozdov@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On 07/28/2015 12:57 PM, Alexander Drozdov wrote: > tpacket_fill_skb() can return a negative value (-errno) which > is stored in tp_len variable. In that case the following > condition will be (but shouldn't be) true: > > tp_len > dev->mtu + dev->hard_header_len > > as dev->mtu and dev->hard_header_len are both unsigned. > > That may lead to just returning an incorrect EMSGSIZE errno > to the user. > > Signed-off-by: Alexander Drozdov <al.drozdov@gmail.com> Looks good to me, thanks! Acked-by: Daniel Borkmann <daniel@iogearbox.net> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2015-07-28 at 13:07 +0200, Daniel Borkmann wrote: > On 07/28/2015 12:57 PM, Alexander Drozdov wrote: > > tpacket_fill_skb() can return a negative value (-errno) which > > is stored in tp_len variable. In that case the following > > condition will be (but shouldn't be) true: > > > > tp_len > dev->mtu + dev->hard_header_len > > > > as dev->mtu and dev->hard_header_len are both unsigned. > > > > That may lead to just returning an incorrect EMSGSIZE errno > > to the user. > > > > Signed-off-by: Alexander Drozdov <al.drozdov@gmail.com> > > Looks good to me, thanks! > > Acked-by: Daniel Borkmann <daniel@iogearbox.net> > -- Fixes: 52f1454f629fa ("packet: allow to transmit +4 byte in TX_RING slot for VLAN case") -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Alexander Drozdov <al.drozdov@gmail.com> Date: Tue, 28 Jul 2015 13:57:01 +0300 > tpacket_fill_skb() can return a negative value (-errno) which > is stored in tp_len variable. In that case the following > condition will be (but shouldn't be) true: > > tp_len > dev->mtu + dev->hard_header_len > > as dev->mtu and dev->hard_header_len are both unsigned. > > That may lead to just returning an incorrect EMSGSIZE errno > to the user. > > Signed-off-by: Alexander Drozdov <al.drozdov@gmail.com> Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index c9e8741..d1d3625 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -2403,7 +2403,8 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) } tp_len = tpacket_fill_skb(po, skb, ph, dev, size_max, proto, addr, hlen); - if (tp_len > dev->mtu + dev->hard_header_len) { + if (likely(tp_len >= 0) && + tp_len > dev->mtu + dev->hard_header_len) { struct ethhdr *ehdr; /* Earlier code assumed this would be a VLAN pkt, * double-check this now that we have the actual
tpacket_fill_skb() can return a negative value (-errno) which is stored in tp_len variable. In that case the following condition will be (but shouldn't be) true: tp_len > dev->mtu + dev->hard_header_len as dev->mtu and dev->hard_header_len are both unsigned. That may lead to just returning an incorrect EMSGSIZE errno to the user. Signed-off-by: Alexander Drozdov <al.drozdov@gmail.com> --- net/packet/af_packet.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)