diff mbox

af_packet: check loop for greater than zero in tpacket_fill_skb

Message ID 4FD4F494.6020102@iogearbox.net
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

danborkmann@iogearbox.net June 10, 2012, 7:25 p.m. UTC
It could be more safe to check the 'to_write' for 'greater than zero'
instead for 'not zero'. 'to_write' is of type int and subtraction operations
are performed on it, so in the case of malformed values that are
subtracted from 'to_write', it could become less than zero, which is then
interpreted as 'not zero' in the while condition, thus the loop won't
return as expected.

Signed-off-by: Daniel Borkmann <daniel.borkmann@tik.ee.ethz.ch>
---
 net/packet/af_packet.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

--
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

Comments

David Miller June 11, 2012, 3:10 a.m. UTC | #1
From: Daniel Borkmann <danborkmann@iogearbox.net>
Date: Sun, 10 Jun 2012 21:25:08 +0200

> It could be more safe to check the 'to_write' for 'greater than zero'
> instead for 'not zero'. 'to_write' is of type int and subtraction operations
> are performed on it, so in the case of malformed values that are
> subtracted from 'to_write', it could become less than zero, which is then
> interpreted as 'not zero' in the while condition, thus the loop won't
> return as expected.
> 
> Signed-off-by: Daniel Borkmann <daniel.borkmann@tik.ee.ethz.ch>

I hate this kind of change.

The implication is that there are bugs in the loop that can cause
the value to go negative.

Unless you can show that this actually happens, leave this clean
test alone.
--
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 0f66174..3e53680 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2027,7 +2027,7 @@  static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
 	skb->truesize += to_write;
 	atomic_add(to_write, &po->sk.sk_wmem_alloc);
 
-	while (likely(to_write)) {
+	while (likely(to_write > 0)) {
 		nr_frags = skb_shinfo(skb)->nr_frags;
 
 		if (unlikely(nr_frags >= MAX_SKB_FRAGS)) {