Re: 9pfs double kfree

[Latchesar Ionkov]

Hi,

The first issue is a bug, I'll send a patch in a few minutes.

The second one is not a bug. kfree on line 477 is freeing the
strucuture allocated by v9fs_t_walk. v9fs_t_create call overwrites
fcall (after the call it is either NULL or points to another memory
area) so the kfree on line 294 (or line 301 in case of error) don't do
double-free.

There is another bug in the second function though. If v9fs_get_idpool
fails, the execution goes to error label and kfree is called on the
uninitialized fcall. I'll include a fix of this bug too.

Thanks,
    Lucho

On 3/6/06, Dave Jones <davej@xxxxxxxxxx> wrote:
> Probably the first of many found with Coverity.
>
> This is kfree'd outside of both arms of the if condition already,
> so fall through and free it just once.
>
> Second variant is double-nasty, it deref's the free'd fcall
> before it tries to free it a second time.
>
> (I wish we had a kfree variant that NULL'd the target when it was free'd)
>
> Coverity bugs: 987, 986
>
> Signed-off-by: Dave Jones <davej@xxxxxxxxxx>
>
>
> --- linux-2.6.15.noarch/fs/9p/vfs_super.c~      2006-03-06 01:53:38.000000000 -0500
> +++ linux-2.6.15.noarch/fs/9p/vfs_super.c       2006-03-06 01:54:36.000000000 -0500
> @@ -156,7 +156,6 @@ static struct super_block *v9fs_get_sb(s
>         stat_result = v9fs_t_stat(v9ses, newfid, &fcall);
>         if (stat_result < 0) {
>                 dprintk(DEBUG_ERROR, "stat error\n");
> -               kfree(fcall);
>                 v9fs_t_clunk(v9ses, newfid);
>         } else {
>                 /* Setup the Root Inode */
> --- linux-2.6.15.noarch/fs/9p/vfs_inode.c~      2006-03-06 01:57:05.000000000 -0500
> +++ linux-2.6.15.noarch/fs/9p/vfs_inode.c       2006-03-06 01:58:05.000000000 -0500
> @@ -274,7 +274,6 @@ v9fs_create(struct v9fs_session_info *v9
>                 PRINT_FCALL_ERROR("clone error", fcall);
>                 goto error;
>         }
> -       kfree(fcall);
>
>         err = v9fs_t_create(v9ses, fid, name, perm, mode, &fcall);
>         if (err < 0) {
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/