linux-kernel - Re: [PATCH] staging: rtl8188eu: os_dep: usb_intf.c: Fix for possible null pointer dereference

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]

Date:	Wed, 21 May 2014 10:24:50 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	josh@...htriplett.org
Cc:	Larry Finger <Larry.Finger@...inger.net>,
	devel@...verdev.osuosl.org,
	Rickard Strandqvist <rickard_strandqvist@...ctrumdigital.se>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	Manu Gupta <manugupt1@...il.com>
Subject: Re: [PATCH] staging: rtl8188eu: os_dep: usb_intf.c:  Fix for
 possible null pointer dereference

On Tue, May 20, 2014 at 04:57:56PM -0700, josh@...htriplett.org wrote:
> On Tue, May 20, 2014 at 06:26:51PM -0500, Larry Finger wrote:
> > On 05/20/2014 04:31 PM, Rickard Strandqvist wrote:
> > >There is otherwise a risk of a possible null pointer dereference.
> > >
> > >Was largely found by using a static code analysis program called cppcheck.
> > >
> > >Signed-off-by: Rickard Strandqvist <rickard_strandqvist@...ctrumdigital.se>
> > >---
> > >  drivers/staging/rtl8188eu/os_dep/usb_intf.c |  127 ++++++++++++++-------------
> > >  1 file changed, 66 insertions(+), 61 deletions(-)
> > 
> > Although cppcheck's analysis is correct, pointer padapter can never
> > be null in any of these routines. In routine rtw_drv_init(),
> > rtw_usb_if1_init() is called to allocate memory for struct adapter
> > for the driver. If that fails, none of these other routines will
> > ever be called as the driver will not be loaded.
> > 
> > If it is deemed better to fill the code with needless tests because
> > some static checker points out a place like this, that is OK with
> > me, but I do not see the point.
> 
> If it's an invariant of the code that padapter cannot ever be NULL, then
> that seems perfectly fine to rely on, and the tool just needs fixing or
> silencing.  At most, it might be helpful to add annotations like GCC's
> "nonnull", if that helps the checker and the compiler generate more
> useful warnings.

The tool is complaining that the existing checks for NULL don't make
sense.  Rickard changed them to cover all the dereferences but really we
should just remove the checks.

Btw, speaking of GCC, it now silently removes inconsistent NULL
checking.  It takes the opposite of the normall kernel programmer
approach of adding checks everywhere.  :P  The glibc people recently
annotated qsort() to say that the function pointer can't be NULL even
though this "worked" in the past.  Hillarity!

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61236

regards,
dan carpenter

--
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/