Re: [CHECKER] null-deref errors for 2.4.7

• Next message: Paul Mundt: "Re: [PATCH] Port tdfxfb to new-style PCI API"
• Previous message: Daniel Phillips: "Re: kernel archives format"
• In reply to: Evan Parker: "[CHECKER] null-deref errors for 2.4.7"
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Evan,

thanks for your report!

On Tue, Jul 24, 2001 at 03:31:26PM -0700, Evan Parker wrote:
> Enclosed are 100 bugs found by an automatic checker that checks
> the internal consistency of code with respect to pointer dereferences and
> comparisons to NULL.
[...]
> The most common pattern the checker found was when a pointer is
> dereferenced in a declaration initialization, and then immidately checked
> against NULL. The following code, one example of this pattern, was found
> copied and pasted almost verbatim in over 20 different locations.

[...]

> Overall, this checker is pretty robust, with a very low false positive
> rate. On kernel 2.4.7, it found 146 possible errors, of which 100 were
> obvious bugs, 18 were false positives, and the rest were unknown (usually
> because the dereference and the check happened too far apart).
> Furthermore, the majority of the false positives were triggered by macro
> expansions that included checks against NULL.
>
>
> # Summary for 2.4.7
> # Total Errors = 100
> # BUGs | File Name
[...]
> 2 | drivers/scsi/tmscsim.c/
[...]
> [BUG]
> drivers/scsi/tmscsim.c:2571:dc390_set_info: ERROR:INTERNAL_NULL2:2557:2571: Checked "pos" against null AFTER dereferencing it! [nbytes = 1] [distance=25]
> int dum = 0;
> char dev;
> PDCB pDCB = pACB->pLinkDCB;
> DC390_IFLAGS
> DC390_AFLAGS
> Start --->
> pos[length] = 0;
>
> DC390_LOCK_IO;
> DC390_LOCK_ACB;
> /* UPPERCASE */
> /* Don't use kernel toupper, because of 2.0.x bug: ctmp unexported */
> while (*pos)
> { if (*pos >='a' && *pos <= 'z') *pos = *pos + 'A' - 'a'; pos++; };
>
> /* We should protect __strtok ! */
> /* spin_lock (strtok_lock); */
>
> /* Remove WS */
> pos = strtok (buffer, " \t:\n=,;");
> Error --->
> if (!pos) goto ok;
>
> next:
> if (!memcmp (pos, "RESET", 5)) goto reset;
> ---------------------------------------------------------
> [BUG]
> drivers/scsi/tmscsim.c:2658:dc390_set_info: ERROR:INTERNAL_NULL2:2653:2658: Checked "pos" against null AFTER passing it to "simple_strtoul"! [nbytes = 1] [distance=26]
> if (!pos) goto ok;
> /* decimal */
> if (pos-1 == pdec)
> {
> int dumold = dum;
> Start --->
> dum = simple_strtoul (pos, &p0, 10) * 10;
> for (; p0-pos > 1; p0--) dum /= 10;
> pDCB->NegoPeriod = (100000/(100*dumold + dum)) >> 2;
> if (pDCB->NegoPeriod < 19) pDCB->NegoPeriod = 19;
> pos = strtok (0, " \t\n:=,;");
> Error --->
> if (!pos) goto ok;
> };
> if (*pos == 'M') pos = strtok (0, " \t\n:=,;");
> if (pDCB->NegoPeriod != olddevmode) needs_inquiry++;
> ---------------------------------------------------------

Both places look correct to me, as the check is after a token has been
searched for with strtok ().
Please contradict if you think I'm wrong.

Regards,

-- 
Kurt Garloff  <garloff@suse.de>                          Eindhoven, NL
GPG key: See mail header, key servers         Linux kernel development
SuSE GmbH, Nuernberg, DE                                SCSI, Security

• application/pgp-signature attachment: stored

• Next message: Paul Mundt: "Re: [PATCH] Port tdfxfb to new-style PCI API"
• Previous message: Daniel Phillips: "Re: kernel archives format"
• In reply to: Evan Parker: "[CHECKER] null-deref errors for 2.4.7"
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

This archive was generated by hypermail 2b29 : Tue Jul 31 2001 - 21:00:42 EST