Re: [2.6 patch] drivers/ieee1394/raw1394.c: fix a NULL pointer dereference

[Jesper Juhl <jesper.juhl@gmail.com>]

On 11/21/05, Adrian Bunk <bunk@stusta.de> wrote:
> The coverity checker spotted that this was a NULL pointer dereference in
> the "if (copy_from_user(...))" case.
>
>
> Signed-off-by: Adrian Bunk <bunk@stusta.de>
>
> --- linux-2.6.15-rc1-mm2-full/drivers/ieee1394/raw1394.c.old    2005-11-20 22:08:57.000000000 +0100
> +++ linux-2.6.15-rc1-mm2-full/drivers/ieee1394/raw1394.c        2005-11-20 22:09:34.000000000 +0100
> @@ -2166,7 +2166,8 @@
>                         }
>                 }
>         }
> -       kfree(cache->filled_head);
> +       if(cache->filled_head)
> +               kfree(cache->filled_head);
>         kfree(cache);
>
Hmmm,  kfree() deals with NULL pointers just fine, so there's no
problem if cache->filled_head is NULL. There is, however, a NULL
pointer deref problem if `cache' is NULL, but that's not what your
patch checks for.

Shouldn't your patch be doing something like this (that is if cache
can ever be NULL at this point)?  :

--- linux-2.6.15-rc2-orig/drivers/ieee1394/raw1394.c    2005-11-20
22:25:27.000000000 +0100
+++ linux-2.6.15-rc2/drivers/ieee1394/raw1394.c 2005-11-21
00:33:34.000000000 +0100
@@ -2171,8 +2171,10 @@ static int modify_config_rom(struct file
                        }
                }
        }
-       kfree(cache->filled_head);
-       kfree(cache);
+       if (cache) {
+               kfree(cache->filled_head);
+               kfree(cache);
+       }

        if (ret >= 0) {
                /* we have to free the request, because we queue no response,


--
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html