diff mbox

[net-next] packet: make packet_snd fail on len smaller than l2 header

Message ID 1416416165-24697-1-git-send-email-willemb@google.com
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Willem de Bruijn Nov. 19, 2014, 4:56 p.m. UTC
From: Willem de Bruijn <willemb@google.com>

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.
Also rate limit the warning in tpacket_snd.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 net/packet/af_packet.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

Comments

Daniel Borkmann Nov. 19, 2014, 5:16 p.m. UTC | #1
On 11/19/2014 05:56 PM, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> 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.
> Also rate limit the warning in tpacket_snd.
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>

Ok, one thing below though, otherwise:

Acked-by: Daniel Borkmann <dborkman@redhat.com>

> ---
>   net/packet/af_packet.c | 30 ++++++++++++++++++++++--------
>   1 file changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 4cd13d8..0d3cb44 100644
> --- 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)
>   	sock_wfree(skb);
>   }
>
> +static bool ll_header_truncated(int len, struct net_device *dev)
> +{

const struct net_device *dev

[ if at it, I'd perhaps also swap the arg order ]

> +	/* net device doesn't like empty head */
> +	if (unlikely(len <= dev->hard_header_len)) {
> +		net_warn_ratelimited("%s: packet size is too short (%d < %d)\n",
> +				     current->comm, len, dev->hard_header_len);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
>   static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
>   		void *frame, struct net_device *dev, int size_max,
>   		__be16 proto, unsigned char *addr, int hlen)
> @@ -2170,12 +2182,8 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
>   		if (unlikely(err < 0))
>   			return -EINVAL;
>   	} else if (dev->hard_header_len) {
> -		/* net device doesn't like empty head */
> -		if (unlikely(tp_len <= dev->hard_header_len)) {
> -			pr_err("packet size is too short (%d < %d)\n",
> -			       tp_len, dev->hard_header_len);
> +		if (ll_header_truncated(tp_len, dev))
>   			return -EINVAL;
> -		}
>
>   		skb_push(skb, dev->hard_header_len);
>   		err = skb_store_bits(skb, 0, data,
> @@ -2500,9 +2508,15 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
>   	skb_set_network_header(skb, reserve);
>
>   	err = -EINVAL;
> -	if (sock->type == SOCK_DGRAM &&
> -	    (offset = dev_hard_header(skb, dev, ntohs(proto), addr, NULL, len)) < 0)
> -		goto out_free;
> +	if (sock->type == SOCK_DGRAM) {
> +		offset = dev_hard_header(skb, dev, ntohs(proto), addr, NULL,
> +					 len);
> +		if (unlikely(offset) < 0)
> +			goto out_free;
> +	} else {
> +		if (ll_header_truncated(len, dev))
> +			goto out_free;
> +	}
>
>   	/* Returns -EFAULT on error */
>   	err = skb_copy_datagram_from_iovec(skb, offset, msg->msg_iov, 0, len);
>
--
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
Willem de Bruijn Nov. 19, 2014, 6:07 p.m. UTC | #2
>> ---
>>   net/packet/af_packet.c | 30 ++++++++++++++++++++++--------
>>   1 file changed, 22 insertions(+), 8 deletions(-)
>>
>> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
>> index 4cd13d8..0d3cb44 100644
>> --- 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)
>>         sock_wfree(skb);
>>   }
>>
>> +static bool ll_header_truncated(int len, struct net_device *dev)
>> +{
>
>
> const struct net_device *dev
>
> [ if at it, I'd perhaps also swap the arg order ]

Thanks, Daniel. I'll resubmit with both changes.
--
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 mbox

Patch

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 4cd13d8..0d3cb44 100644
--- 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)
 	sock_wfree(skb);
 }
 
+static bool ll_header_truncated(int len, struct net_device *dev)
+{
+	/* net device doesn't like empty head */
+	if (unlikely(len <= dev->hard_header_len)) {
+		net_warn_ratelimited("%s: packet size is too short (%d < %d)\n",
+				     current->comm, len, dev->hard_header_len);
+		return true;
+	}
+
+	return false;
+}
+
 static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
 		void *frame, struct net_device *dev, int size_max,
 		__be16 proto, unsigned char *addr, int hlen)
@@ -2170,12 +2182,8 @@  static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
 		if (unlikely(err < 0))
 			return -EINVAL;
 	} else if (dev->hard_header_len) {
-		/* net device doesn't like empty head */
-		if (unlikely(tp_len <= dev->hard_header_len)) {
-			pr_err("packet size is too short (%d < %d)\n",
-			       tp_len, dev->hard_header_len);
+		if (ll_header_truncated(tp_len, dev))
 			return -EINVAL;
-		}
 
 		skb_push(skb, dev->hard_header_len);
 		err = skb_store_bits(skb, 0, data,
@@ -2500,9 +2508,15 @@  static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
 	skb_set_network_header(skb, reserve);
 
 	err = -EINVAL;
-	if (sock->type == SOCK_DGRAM &&
-	    (offset = dev_hard_header(skb, dev, ntohs(proto), addr, NULL, len)) < 0)
-		goto out_free;
+	if (sock->type == SOCK_DGRAM) {
+		offset = dev_hard_header(skb, dev, ntohs(proto), addr, NULL,
+					 len);
+		if (unlikely(offset) < 0)
+			goto out_free;
+	} else {
+		if (ll_header_truncated(len, dev))
+			goto out_free;
+	}
 
 	/* Returns -EFAULT on error */
 	err = skb_copy_datagram_from_iovec(skb, offset, msg->msg_iov, 0, len);