linux-kernel - Re: [PATCH][RESEND] Fix a potential NULL pointer deref in the aic7xxx, ahc_print

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <46B5EFE7.6090008@scsiguy.com>
Date:	Sun, 05 Aug 2007 09:42:31 -0600
From:	"Justin T. Gibbs" <gibbs@...iguy.com>
To:	Jesper Juhl <jesper.juhl@...il.com>
CC:	James Bottomley <James.Bottomley@...eleye.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	James Bottomley <jejb@...nel.org>, linux-scsi@...r.kernel.org,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH][RESEND] Fix a potential NULL pointer deref in the aic7xxx,
 ahc_print_register() function

All of this logic was simplified back in '05 in the BSD drivers by adding
this to the top of the function:

        u_int   dummy_column;
  
        if (cur_column == NULL) {
                dummy_column = 0;
                cur_column = &dummy_column;
        }

and then stripping out the cur_column == NULL checks in the routine.

--
Justin

Jesper Juhl wrote:
> On 04/08/07, James Bottomley <James.Bottomley@...eleye.com> wrote:
>> On Sat, 2007-08-04 at 20:30 +0200, Jesper Juhl wrote:
>>> (resend of patch previously submitted on 28-Jul-2007 23:06)
>>>
>>>
>>> Ehlo,
>>>
>>> The Coverity checker noticed that we have a potential NULL pointer
>>> deref in drivers/scsi/aic7xxx/aic7xxx_core.c::ahc_print_register().
>>> This patch handles it by adding the same test against NULL that is
>>> used elsewhere in the same function.
>> It's on my list of things to look at ... but not very high.  I suspect
>> it actually isn't triggerable, but if you can tell me how, it will save
>> me from looking.
>>
> 
> Here's what Coverity reported :
> ...
> 6525 	int
> 6526 	ahc_print_register(ahc_reg_parse_entry_t *table, u_int num_entries,
> 6527 			   const char *name, u_int address, u_int value,
> 6528 			   u_int *cur_column, u_int wrap_point)
> 6529 	{
> 6530 		int	printed;
> 6531 		u_int	printed_mask;
> 6532 	
> 
> Event var_compare_op: Added "cur_column" due to comparison "cur_column != 0"
> Also see events: [var_deref_op]
> At conditional (1): "cur_column != 0" taking false path
> 
> 6533 		if (cur_column != NULL && *cur_column >= wrap_point) {
> 6534 			printf("\n");
> 6535 			*cur_column = 0;
> 6536 		}
> 6537 		printed = printf("%s[0x%x]", name, value);
> 
> At conditional (2): "table == 0" taking true path
> 
> 6538 		if (table == NULL) {
> 6539 			printed += printf(" ");
> 
> Event var_deref_op: Variable "cur_column" tracked as NULL was dereferenced.
> Also see events: [var_compare_op]
> 
> 6540 			*cur_column += printed;
> 6541 			return (printed);
> 6542 		}
> ...
> 
> So it requires a NULL 'table' and a != NULL 'cur_column' to trigger.
> Whether or not that's actually possible I'm not sure, but it seems
> safer to guard against it :)
> 
> 
> By the way; if this can actually be triggered, then
> ahd_print_register() has the same problem.
> 
> 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/