Pekka Enberg <[email protected]> wrote:
>
> This patch fixes a buffer_head leak in the function vxfs_getfsh by
> allocating the buffer before doing sb_bread(). In addition, the patch
> replaces misused SLAB_KERNEL flag with the proper GFP_KERNEL and adds
> a NULL check for sb_bread.
>
Yes, there does seem to be a leak there.
> ===================================================================
> --- 2.6.orig/fs/freevxfs/vxfs_fshead.c 2005-06-28 19:48:12.000000000 +0300
> +++ 2.6/fs/freevxfs/vxfs_fshead.c 2005-06-28 19:48:34.000000000 +0300
> @@ -76,19 +76,22 @@
> vxfs_getfsh(struct inode *ip, int which)
> {
> struct buffer_head *bp;
> + struct vxfs_fsh *fhp;
> +
> + if (!(fhp = kmalloc(sizeof(*fhp), GFP_KERNEL)))
> + goto failed;
>
> bp = vxfs_bread(ip, which);
> - if (buffer_mapped(bp)) {
> - struct vxfs_fsh *fhp;
> + if (!bp || !buffer_mapped(bp))
> + goto failed;
>
> - if (!(fhp = kmalloc(sizeof(*fhp), SLAB_KERNEL)))
> - return NULL;
> - memcpy(fhp, bp->b_data, sizeof(*fhp));
> + memcpy(fhp, bp->b_data, sizeof(*fhp));
>
> - brelse(bp);
> - return (fhp);
> - }
> + brelse(bp);
> + return fhp;
>
> +failed:
> + kfree(fhp);
> return NULL;
> }
>
But your change means that we'll always perform that kmalloc, even if the
buffer came back !buffer_mapped().
<looks>
I don't think sb_bread() can return an unmapped buffer at all.
And sb_bread() can return NULL (I/O error) and we're not checking for that
in there.
Something like this?
diff -puN fs/freevxfs/vxfs_fshead.c~freevxfs-fix-buffer_head-leak fs/freevxfs/vxfs_fshead.c
--- 25/fs/freevxfs/vxfs_fshead.c~freevxfs-fix-buffer_head-leak Tue Jun 28 16:19:53 2005
+++ 25-akpm/fs/freevxfs/vxfs_fshead.c Tue Jun 28 16:24:53 2005
@@ -78,14 +78,16 @@ vxfs_getfsh(struct inode *ip, int which)
struct buffer_head *bp;
bp = vxfs_bread(ip, which);
- if (buffer_mapped(bp)) {
+ if (bp && buffer_mapped(bp)) {
struct vxfs_fsh *fhp;
- if (!(fhp = kmalloc(sizeof(*fhp), SLAB_KERNEL)))
+ if (!(fhp = kmalloc(sizeof(*fhp), GFP_KERNEL))) {
+ put_bh(bp);
return NULL;
+ }
memcpy(fhp, bp->b_data, sizeof(*fhp));
- brelse(bp);
+ put_bh(bp);
return (fhp);
}
_
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
