From: Andrew Morton (uwoynke@merck.com)
Date: Tue Aug 01 2000 - 12:26:45 EEST
Feeding kernel 2.2.17 through `gcc -W' has shown a number of bugs.
I have attached here a proposed patch which fixes a small number of these,
but for the rest I would ask those on the "To:" list to let me know how they
would prefer that these be addressed. I shall then cut a final patch.
Thanks.
acenic.c:973: warning: unsigned value < 0 is always 0
acenic.c:983: warning: unsigned value < 0 is always 0
Error returns from read_eeprom_byte() are not recognised. This
buglet is also present in the 2.4 driver. No patch provided here -
I'll assume that Jes will be sending one through when he updates the
2.4 driver.
hmm.. acenic doesn't have the other fixes applied yet:
The precedence error in ace_timer (5/2*HZ).
dev->priv should be zeroed after allocation (there was a crash
codepath without this).
del_timer_sync() for 2.4.
af_ax25.c:519: warning: unsigned value < 0 is always 0
af_ax25.c:525: warning: unsigned value < 0 is always 0
Bounds checking on ioctl args. Probably not worth fixing.
b1.c:601: warning: unsigned value >= 0 is always 1
b1.c:612: warning: unsigned value >= 0 is always 1
b1dma.c:556: warning: unsigned value >= 0 is always 1
b1dma.c:567: warning: unsigned value >= 0 is always 1
c4.c:659: warning: unsigned value >= 0 is always 1
c4.c:670: warning: unsigned value >= 0 is always 1
t1isa.c:283: warning: unsigned value >= 0 is always 1
t1isa.c:294: warning: unsigned value >= 0 is always 1
Potentially nasty - underindexing of card->MsgBuf[]. Patched here.
br.c:2261: warning: unsigned value < 0 is always 0
br.c:2279: warning: unsigned value < 0 is always 0
br.c:2302: warning: unsigned value < 0 is always 0
br.c:2313: warning: unsigned value < 0 is always 0
Nasty - can result in underindexing of port_info[]. Unprivileged
users can possibly crash the machine. Patched here.
cs46xx.c:2215: warning: unsigned value >= 0 is always 1
cs46xx.c:2246: warning: unsigned value >= 0 is always 1
These are potentially nasty. A hardware-bit-testing loop which
is supposed to have a timeout is in fact potentially infinite.
No patch here because it would be behaviour-altering and I can't
test it.
The fix is to make `end_time' signed. Should use time_after().
epca.c:3836: warning: ordered comparison of pointer with integer zero
epca.c:3848: warning: ordered comparison of pointer with integer zero
I'm not sure what `pointer <= 0' does... But this is a simple
fix to the input parameter validation and I've patched it here.
drivers/video/fbmem.c:478
if (con2fb.console < 0 || con2fb.console > MAX_NR_CONSOLES)
return -EINVAL;
I think this is an off-by-one. Should it be `>= MAX_NR_CONSOLES'?
hp100.c:2920: warning: comparison of promoted ~unsigned with constant
This code:
do {
if (~(hp100_inb( VG_LAN_CFG_1 )& HP100_LINK_UP_ST) ) break;
} while (time_after(time, jiffies));
Is clearly bogus (the `~'). But a fix is behaviour-altering and I
can't test it.
./ip2/i2lib.c:1261: warning: comparisons like X<=Y<=Z do not have their
mathematical meaning
It looks like this code flukes correctness.
iph5526.c:3778: warning: comparisons like X<=Y<=Z do not have their
mathematical meaning
Ditto.
ixj.c:3029: warning: empty body in an if-statement
if (ixj[board].play_mode != -1 && ixj[board].rec_mode != -1);
{
ixj_WriteDSPCommand(0xB002, board); // AEC Stop
}
See the semicolon? This has to be a bug, but a fix is
behaviour-altering and I can't test it.
ll_rw_blk.c:761: warning: unsigned value < 0 is always 0
Here's the code:
/* Can we add it to the end of this request? */
if (req->sector + req->nr_sectors == sector) {
if (latency - req->nr_segments < 0)
break;
The inner test is a no-op. I think we decided it could simply
be removed in the 2.4 case. Is that true here as well?
drivers/sound/emu10k1/main.c:653: warning: unsigned value < 0 is always 0
drivers/sound/emu10k1/main.c:658: warning: unsigned value < 0 is always 0
drivers/sound/emu10k1/main.c:663: warning: unsigned value < 0 is always 0
drivers/sound/emu10k1/main.c:668: warning: unsigned value < 0 is always 0
Four ineffective attempts to check return values.
mm/memory.c:395: warning: unsigned value < 0 is always 0
Here's the code:
if (mm->rss > 0) {
mm->rss -= freed;
if (mm->rss < 0)
mm->rss = 0;
}
The check for RSS underflow is ineffective because it's unsigned.
What are we actually trying to do here?
net/netrom/nr_route.c:625
if (nr_route.ndigis < 0 || nr_route.ndigis > AX25_MAX_DIGIS)
return -EINVAL;
Is this an off-by-one error? Should it be `>= AX25_MAX_DIGIS'?
old_tulip.c:2732: warning: unsigned value >= 0 is always 1
hmmm.. In 2.4 this was a bug because `dummy' was initialised to
-1. Here it's initialised to zero and the logic is subtly different.
Jeff, is this driver broken?
drivers/net/sbni.c:1379: warning: control reaches end of non-void function
This looks broken. Why is the (required) `return crc' commented out?
mm/slab.c:717:
if (offset < 0 || offset > size) {
printk("%sOffset weird %d - %s\n", func_nm, (int) offset, name);
offset = 0;
}
The first part of the comparison is ineffective when size_t is unsigned.
The second part of the comparison seems to be an off-by-one. Should it
be `offset >= size'?
fs/ntfs/support.c:301: warning: `ch' might be used uninitialized in this
function
fs/ntfs/support.c:301: warning: `cl' might be used uninitialized in this
function
The warning is correct. This code looks buggy.
fs/coda/upcall.c:741: warning: unsigned value < 0 is always 0
Ineffective attempt to check an error code.
--- linux-2.2.17pre14/drivers/isdn/avmb1/b1.c Fri Jun 16 23:47:49 2000
+++ linux-akpm/drivers/isdn/avmb1/b1.c Mon Jul 31 22:28:22 2000
@@ -507,7 +507,7 @@
struct sk_buff *skb;
unsigned ApplId;
- unsigned MsgLen;
+ signed MsgLen;
unsigned DataB3Len;
unsigned NCCI;
unsigned WindowSize;
--- linux-2.2.17pre14/drivers/isdn/avmb1/b1dma.c Fri Jun 16 23:47:49 2000
+++ linux-akpm/drivers/isdn/avmb1/b1dma.c Mon Jul 31 22:29:48 2000
@@ -462,7 +462,8 @@
struct capi_ctr *ctrl = cinfo->capi_ctrl;
struct sk_buff *skb;
void *p = dma->recvbuf+4;
- __u32 ApplId, MsgLen, DataB3Len, NCCI, WindowSize;
+ __u32 ApplId, DataB3Len, NCCI, WindowSize;
+ __s32 MsgLen;
__u8 b1cmd = _get_byte(&p);
#ifdef CONFIG_B1DMA_DEBUG
--- linux-2.2.17pre14/net/bridge/br.c Wed Jan 5 05:12:26 2000
+++ linux-akpm/net/bridge/br.c Mon Jul 31 22:32:14 2000
@@ -2258,7 +2258,7 @@
break;
case BRCMD_IF_ENABLE:
bcf.arg1 = br_find_port(bcf.arg1);
- if (bcf.arg1 < 0)
+ if ((signed)bcf.arg1 < 0)
return(bcf.arg1);
case BRCMD_PORT_ENABLE:
if (port_info[bcf.arg1].dev == 0)
@@ -2276,7 +2276,7 @@
break;
case BRCMD_IF_DISABLE:
bcf.arg1 = br_find_port(bcf.arg1);
- if (bcf.arg1 < 0)
+ if ((signed)bcf.arg1 < 0)
return(bcf.arg1);
case BRCMD_PORT_DISABLE:
if (port_info[bcf.arg1].dev == 0)
@@ -2299,7 +2299,7 @@
break;
case BRCMD_SET_IF_PRIORITY:
bcf.arg1 = br_find_port(bcf.arg1);
- if (bcf.arg1 < 0)
+ if ((signed)bcf.arg1 < 0)
return(bcf.arg1);
case BRCMD_SET_PORT_PRIORITY:
if((port_info[bcf.arg1].dev == 0)
@@ -2310,7 +2310,7 @@
break;
case BRCMD_SET_IF_PATH_COST:
bcf.arg1 = br_find_port(bcf.arg1);
- if (bcf.arg1 < 0)
+ if ((signed)bcf.arg1 < 0)
return(bcf.arg1);
case BRCMD_SET_PATH_COST:
if (port_info[bcf.arg1].dev == 0)
--- linux-2.2.17pre14/drivers/isdn/avmb1/c4.c Fri Jun 16 23:47:49 2000
+++ linux-akpm/drivers/isdn/avmb1/c4.c Mon Jul 31 22:46:01 2000
@@ -535,7 +535,8 @@
avmctrl_info *cinfo;
struct sk_buff *skb;
void *p = dma->recvbuf;
- __u32 ApplId, MsgLen, DataB3Len, NCCI, WindowSize;
+ __u32 ApplId, DataB3Len, NCCI, WindowSize;
+ __s32 MsgLen;
__u8 b1cmd = _get_byte(&p);
__u32 cidx;
--- linux-2.2.17pre14/drivers/char/epca.c Fri Jun 16 23:47:49 2000
+++ linux-akpm/drivers/char/epca.c Mon Jul 31 22:56:57 2000
@@ -3833,7 +3833,7 @@
case 5:
board.port = (unsigned char *)ints[index];
- if (board.port <= 0)
+ if ((signed long)board.port <= 0)
{
printk(KERN_ERR "<Error> - epca_setup: Invalid io port 0x%x\n", (unsigned int)board.port);
invalid_lilo_config = 1;
@@ -3845,7 +3845,7 @@
case 6:
board.membase = (unsigned char *)ints[index];
- if (board.membase <= 0)
+ if ((signed long)board.membase <= 0)
{
printk(KERN_ERR "<Error> - epca_setup: Invalid memory base 0x%x\n",(unsigned int)board.membase);
invalid_lilo_config = 1;
--- linux-2.2.17pre14/drivers/isdn/avmb1/t1isa.c Fri Jun 16 23:47:49 2000
+++ linux-akpm/drivers/isdn/avmb1/t1isa.c Tue Aug 1 00:06:18 2000
@@ -190,7 +190,7 @@
struct sk_buff *skb;
unsigned ApplId;
- unsigned MsgLen;
+ signed MsgLen;
unsigned DataB3Len;
unsigned NCCI;
unsigned WindowSize;
This archive was generated by hypermail 2b29 : Tue Aug 01 2000 - 12:36:45 EEST