Skip to contentSkip to conversation

Uninitialized variable in drivers/net/wan/syncppp.c

PATCH In review Eric Sesterhenn 2 participants 4 messages Aug 31, 2006 – Sep 1, 2006 Network drivers

lore
Share & tools
lore

Eric Sesterhenn

author
#0 lore raw

PATCH 1/1 Uninitialized variable in drivers/net/wan/syncppp.c

hi,

this was spotted by coverity (id #891), when
len is equal to 4, we dont call sppp_lcp_conf_parse_options(), to initialize rmagic.

Signed-off-by Eric Sesterhenn <[email protected]>
drivers/net/wan/syncppp.c renamed from drivers/net/wan/syncppp.c.orig +3 -2
@@ -505,14 +505,15 @@ static void sppp_lcp_input (struct sppp 
 			skb->len, h);
 		break;
 	case LCP_CONF_REQ:
-		if (len < 4) {
+		if (len <= 4) {
 			if (sp->pp_flags & PP_DEBUG)
 				printk (KERN_DEBUG"%s: invalid lcp configure request packet length: %d bytes\n",
 					dev->name, len);
 			break;
 		}
-		if (len>4 && !sppp_lcp_conf_parse_options (sp, h, len, &rmagic))
+		if (!sppp_lcp_conf_parse_options (sp, h, len, &rmagic))
 			goto badreq;
+
 		if (rmagic == sp->lcp.magic) {
 			/* Local and remote magics equal -- loopback? */
 			if (sp->pp_loopcnt >= MAXALIVECNT*5) {
#1 lore raw
Quoted 10 lines · Eric Sesterhenn
drivers/net/wan/syncppp.c renamed from drivers/net/wan/syncppp.c.orig +1 -1
@@ -505,14 +505,15 @@ static void sppp_lcp_input (struct sppp 
 			skb->len, h);
 		break;
 	case LCP_CONF_REQ:
-		if (len < 4) {
+		if (len <= 4) {
 			if (sp->pp_flags & PP_DEBUG)
 				printk (KERN_DEBUG"%s: invalid lcp configure request packet length: %d bytes\n",

drivers/net/wan/syncppp.c static void sppp_lcp_input (struct sppp · line 519

Quoted 10 lines
drivers/net/wan/syncppp.c renamed from drivers/net/wan/syncppp.c.orig +2 -1
 					dev->name, len);
 			break;
 		}
-		if (len>4 && !sppp_lcp_conf_parse_options (sp, h, len, &rmagic))
+		if (!sppp_lcp_conf_parse_options (sp, h, len, &rmagic))
 			goto badreq;
+
 		if (rmagic == sp->lcp.magic) {
 			/* Local and remote magics equal -- loopback? */
 			if (sp->pp_loopcnt >= MAXALIVECNT*5) {

This is not correct.

Quoted 1 line

From RFC1661:

Valid LCP configuration requests can have zero options (len == 4). If the magic number option is not included in the LCP CFG REQ, then the magic number should be treated as zero.

The correct fix is to initialize rmagic to zero before the if (len>4 && !sppp_lcp_conf_parse_options()) line.

--
Paul

author
#2 lore raw

hi,

Quoted 6 lines · Paul Fulghum

Valid LCP configuration requests can have zero options (len == 4). If the magic number option is not included in the LCP CFG REQ, then the magic number should be treated as zero.

The correct fix is to initialize rmagic to zero before the if (len>4 && !sppp_lcp_conf_parse_options()) line.

Thanks for clarification. Here is an updated patch, which has the advantage of also silencing the gcc warning.

For len equal to 4, we never call sppp_lcp_conf_parse_options(), therefore rmagic does not get initialized.

Signed-off-by Eric Sesterhenn <[email protected]>
drivers/net/wan/syncppp.c renamed from drivers/net/wan/syncppp.c.orig +1 -1
@@ -469,7 +469,7 @@ static void sppp_lcp_input (struct sppp 
 	struct net_device *dev = sp->pp_if;
 	int len = skb->len;
 	u8 *p, opt[6];
-	u32 rmagic;
+	u32 rmagic = 0;
 
 	if (!pskb_may_pull(skb, sizeof(struct lcp_header))) {
 		if (sp->pp_flags & PP_DEBUG)
Acked-by
#3 lore raw
Quoted 11 lines · Eric Sesterhenn

Thanks for clarification. Here is an updated patch, which has the advantage of also silencing the gcc warning.

For len equal to 4, we never call sppp_lcp_conf_parse_options(), therefore rmagic does not get initialized.

Signed-off-by Eric Sesterhenn <[email protected]>

drivers/net/wan/syncppp.c static void sppp_lcp_input (struct sppp · line 475

Quoted 8 lines
drivers/net/wan/syncppp.c renamed from drivers/net/wan/syncppp.c.orig +1 -1
@@ -469,7 +469,7 @@ static void sppp_lcp_input (struct sppp 
 	struct net_device *dev = sp->pp_if;
 	int len = skb->len;
 	u8 *p, opt[6];
-	u32 rmagic;
+	u32 rmagic = 0;
 
 	if (!pskb_may_pull(skb, sizeof(struct lcp_header))) {
 		if (sp->pp_flags & PP_DEBUG)
Acked-by Paul Fulghum <[email protected]>