>Number:         32193
>Category:       kern
>Synopsis:       vop_strategy gets broken struct buf's passed by genfs/bread, possible memory leakage
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Nov 29 23:29:00 +0000 2005
>Originator:     Reinoud Zandijk
>Release:        NetBSD 2.99.16/3.99.11
>Organization:
NetBSD
>Environment:
Kernel filingsystem development.
	
	

     $NetBSD: vfs_bio.c,v 1.146 2005/06/09 02:19:59 atatat Exp $
     $NetBSD: genfs_vnops.c,v 1.109 2005/11/12 22:29:53 yamt Exp $

Architecture: i386
Machine: i386
>Description:

When implementing the UDF filingsystem in the NetBSD kernel, i stumbled on 
aparently some rare problems.

UDF's VOP_STRATEGY() gets calls from VOP_READ() using bread() on the vnode 
and from genfs's {get,put}_pages. Both buffers are are not according to the 
spec.

Problems might also occure in write equivalents though i've only studied 
read functions.


VOP_STRATEGY buffers
--------------------
vop_strategy buffers are passed from genfs in 
sys/miscfs/genfs/genfs_vnops.c:836's VOP_TRATEGY call and created at either 
line 673 or at line 810 of the same file. In the buffer `mbp' created at 
line 673, all seems OK but at the buffer `bp' created at line 810, 
bp->b_bufsize is not initialised and thus ZERO!!!! quite a violation.

   {
     s = splbio();
     bp = pool_get(&bufpool, PR_WAITOK);
     splx(s);
     BUF_INIT(bp);
     bp->b_data = (char *)kva + offset - startoffset;
     bp->b_resid = bp->b_bcount = iobytes;
     bp->b_flags = B_BUSY|B_READ|B_CALL|B_ASYNC;
     bp->b_iodone = uvm_aio_biodone1;
     bp->b_vp = vp;
     bp->b_proc = NULL;
   };

This also brings the question as to where these buf `bp' that are only 
created when the `mbp' needs to be fragmented are ever released... i can't 
find it. `mbp' is released at line 864

        s = splbio();
        pool_put(&bufpool, mbp);
        splx(s);

and the pages are mapped out in the following line:
        uvm_pagermapout(kva, npages);

pages, the `bp' are still pointing at... Aparently this code is written so 
that `mbp' can be waited for by `biowait' when all `bp' fragments are 
ready. But the struct buf's claimed from genfs's own pool are not returned 
to the pool. Each time this fragmented scheme is used, memory gets lost.


bread buffers
-------------
vop_strategy buffers are passed from bread() in sys/kern/vfs_bio.c's 
bio_doread() at line 597's VOP_STRATEGY().

These buffers are claimed/looked up just before in line 577's getblk(). 
When passed to UDF's vop_strategy() bp->b_resid is undefined though mostly 
ZERO. Also not according to the struct buf's specs wich would suggest the 
number of bytes to be read/written in/from the buffer to be bp->b_resid.


Other filingsystems
----------
Filingsystems seem to cope with it by passing the buffers directly to the 
device layer that aparently ignores most of the buf contents and only 
reacts to bp->b_count.

Filingsystems that do care about the buffer contents are also only looking 
at bp->b_count.

>How-To-Repeat:
Write a filingsystem :-/
	
>Fix:
Initialise bp->b_bufsize to iobytes in genfs_vnops.c around line 810
     ....
     BUF_INIT(bp);
     bp->b_data = (char *)kva + offset - startoffset;
     bp->b_resid = bp->b_bcount = bp->b_bufsize = iobytes;
     bp->b_flags = B_BUSY|B_READ|B_CALL|B_ASYNC;
     bp->b_iodone = uvm_aio_biodone1;
     bp->b_vp = vp;
     bp->b_proc = NULL;
     ....

Initialise bp->b_resid to size in vfs_bio.c in bio_doread() and write 
companion.

>Unformatted: