Re: [Patch] Possible NULL pointer dereference in fs/configfs/dir.c

[Eric Sesterhenn <snakebyte@gmx.de>]

hi,

On Wed, 2006-03-22 at 15:27 -0800, Joel Becker wrote:
> On Thu, Mar 23, 2006 at 12:05:29AM +0100, Eric Sesterhenn wrote:
> > this fixes coverity bug #845, if group is NULL,
> > we dereference it when setting up dentry.
> 
> 	Is the converity checker merly looking at in-function patterns?

afaik it also looks what the functions which get called do. If you call
a function that might free a pointer you pass, it warns if you use
it afterwards.

> Where can I access the bug report (sorry for the question).

I would guess scan-admin@coverity.com 

> 	group cannot be null here, we aren't called any other way.  So
> while you are correct that the code below is needed in the presence of a
> NULL group, really the "if (group" isn't necessary, just the "if
> (group->default_groups)".  I could even BUG_ON() if you'd like.

I would then propose the following patch, so the check can be
removed for people who like small kernels. I dont think gcc notices
that all callers use non-NULL values and optimizes it away.

--- linux-2.6.16/fs/configfs/dir.c.orig	2006-03-23 00:31:16.000000000 +0100
+++ linux-2.6.16/fs/configfs/dir.c	2006-03-23 00:32:07.000000000 +0100
@@ -504,7 +504,9 @@ static int populate_groups(struct config
 	int ret = 0;
 	int i;
 
-	if (group && group->default_groups) {
+	BUG_ON(!group);		/* group == NULL is not allowed */
+	
+	if (group->default_groups) {
 		/* FYI, we're faking mkdir here
 		 * I'm not sure we need this semaphore, as we're called
 		 * from our parent's mkdir.  That holds our parent's