Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757024AbXHEQF2 (ORCPT ); Sun, 5 Aug 2007 12:05:28 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752589AbXHEQFN (ORCPT ); Sun, 5 Aug 2007 12:05:13 -0400 Received: from ns1.scsiguy.com ([70.89.174.89]:53368 "EHLO ns1.scsiguy.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752073AbXHEQFL (ORCPT ); Sun, 5 Aug 2007 12:05:11 -0400 X-Greylist: delayed 1299 seconds by postgrey-1.27 at vger.kernel.org; Sun, 05 Aug 2007 12:05:11 EDT Message-ID: <46B5EFE7.6090008@scsiguy.com> Date: Sun, 05 Aug 2007 09:42:31 -0600 From: "Justin T. Gibbs" User-Agent: Thunderbird 2.0.0.6 (Windows/20070728) MIME-Version: 1.0 To: Jesper Juhl CC: James Bottomley , Andrew Morton , James Bottomley , linux-scsi@vger.kernel.org, Linux Kernel Mailing List Subject: Re: [PATCH][RESEND] Fix a potential NULL pointer deref in the aic7xxx, ahc_print_register() function References: <200708042030.52405.jesper.juhl@gmail.com> <1186256627.3439.22.camel@localhost.localdomain> <9a8748490708050836m20b5dd38gf6a8968cd4b106f9@mail.gmail.com> In-Reply-To: <9a8748490708050836m20b5dd38gf6a8968cd4b106f9@mail.gmail.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2518 Lines: 80 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 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/