[patch] caif: fix a signedness bug in cfpkt_iterate()

Subsystems: networking [general], the rest

STALE4264d

5 messages, 3 authors, 2015-02-20 · open the first message on its own page

[patch] caif: fix a signedness bug in cfpkt_iterate()

From: Dan Carpenter <hidden>
Date: 2015-02-19 09:13:13

The cfpkt_iterate() function can return -EPROTO on error, but the
function is a u16 so the negative value gets truncated to a positive
unsigned short.  This causes a static checker warning.

The only caller which might care is cffrml_receive(), when it's checking
the frame checksum.  I modified cffrml_receive() so that it never says
-EPROTO is a valid checksum.

Also this isn't ever going to be inlined so I removed the "inline".

Signed-off-by: Dan Carpenter <redacted>
diff --git a/include/net/caif/cfpkt.h b/include/net/caif/cfpkt.h
index 1c1ad46..fe328c5 100644
--- a/include/net/caif/cfpkt.h
+++ b/include/net/caif/cfpkt.h
@@ -171,7 +171,7 @@ struct cfpkt *cfpkt_split(struct cfpkt *pkt, u16 pos);
  * @return    Checksum of buffer.
  */
 
-u16 cfpkt_iterate(struct cfpkt *pkt,
+int cfpkt_iterate(struct cfpkt *pkt,
 		u16 (*iter_func)(u16 chks, void *buf, u16 len),
 		u16 data);
 
diff --git a/net/caif/cfpkt_skbuff.c b/net/caif/cfpkt_skbuff.c
index 1be0b52..f6c3b21 100644
--- a/net/caif/cfpkt_skbuff.c
+++ b/net/caif/cfpkt_skbuff.c
@@ -255,9 +255,9 @@ inline u16 cfpkt_getlen(struct cfpkt *pkt)
 	return skb->len;
 }
 
-inline u16 cfpkt_iterate(struct cfpkt *pkt,
-			 u16 (*iter_func)(u16, void *, u16),
-			 u16 data)
+int cfpkt_iterate(struct cfpkt *pkt,
+		  u16 (*iter_func)(u16, void *, u16),
+		  u16 data)
 {
 	/*
 	 * Don't care about the performance hit of linearizing,
diff --git a/net/caif/cffrml.c b/net/caif/cffrml.c
index 8bc7caa..434ba85 100644
--- a/net/caif/cffrml.c
+++ b/net/caif/cffrml.c
@@ -84,7 +84,7 @@ static int cffrml_receive(struct cflayer *layr, struct cfpkt *pkt)
 	u16 tmp;
 	u16 len;
 	u16 hdrchks;
-	u16 pktchks;
+	int pktchks;
 	struct cffrml *this;
 	this = container_obj(layr);
 

RE: [patch] caif: fix a signedness bug in cfpkt_iterate()

From: David Laight <hidden>
Date: 2015-02-19 09:49:37

From:  Dan Carpenter
...
Also this isn't ever going to be inlined so I removed the "inline".
...
-inline u16 cfpkt_iterate(struct cfpkt *pkt,
-			 u16 (*iter_func)(u16, void *, u16),
-			 u16 data)
static ??
quoted hunk
+int cfpkt_iterate(struct cfpkt *pkt,
+		  u16 (*iter_func)(u16, void *, u16),
+		  u16 data)
	David

Re: [patch] caif: fix a signedness bug in cfpkt_iterate()

From: Dan Carpenter <hidden>
Date: 2015-02-19 10:00:10

On Thu, Feb 19, 2015 at 09:49:00AM +0000, David Laight wrote:
From:  Dan Carpenter
...
quoted
Also this isn't ever going to be inlined so I removed the "inline".
...
quoted
-inline u16 cfpkt_iterate(struct cfpkt *pkt,
-			 u16 (*iter_func)(u16, void *, u16),
-			 u16 data)
static ??
There are some words missing in this sentence.


We could move this function to the header file and make it static inline
if we wanted but checksums aren't used on the fast path anyway so that's
a bit pointless.

regards,
dan carpenter

RE: [patch] caif: fix a signedness bug in cfpkt_iterate()

From: David Laight <hidden>
Date: 2015-02-19 10:06:44

From: Dan Carpenter 
On Thu, Feb 19, 2015 at 09:49:00AM +0000, David Laight wrote:
quoted
From:  Dan Carpenter
...
quoted
Also this isn't ever going to be inlined so I removed the "inline".
...
quoted
-inline u16 cfpkt_iterate(struct cfpkt *pkt,
-			 u16 (*iter_func)(u16, void *, u16),
-			 u16 data)
static ??
There are some words missing in this sentence.
I should have drunk some coffee first.

	David

Re: [patch] caif: fix a signedness bug in cfpkt_iterate()

From: David Miller <davem@davemloft.net>
Date: 2015-02-20 22:35:29

From: Dan Carpenter <redacted>
Date: Thu, 19 Feb 2015 12:13:13 +0300
The cfpkt_iterate() function can return -EPROTO on error, but the
function is a u16 so the negative value gets truncated to a positive
unsigned short.  This causes a static checker warning.

The only caller which might care is cffrml_receive(), when it's checking
the frame checksum.  I modified cffrml_receive() so that it never says
-EPROTO is a valid checksum.

Also this isn't ever going to be inlined so I removed the "inline".

Signed-off-by: Dan Carpenter <redacted>
Applied, thanks Dan.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help