Re: kern/46282: 6.0_BETA crash: msdosfs_bmap -> pcbmap -> bread -> bio_doread

["J. Hannken-Illjes" <hannken%eis.cs.tu-bs.de@localhost>]

The following reply was made to PR kern/46282; it has been noted by GNATS.

From: "J. Hannken-Illjes" <hannken%eis.cs.tu-bs.de@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: Chuck Silvers <chuq%chuq.com@localhost>
Subject: Re: kern/46282: 6.0_BETA crash: msdosfs_bmap -> pcbmap -> bread -> 
bio_doread
Date: Sun, 1 Apr 2012 11:33:01 +0200

 I agree, from a quick look adosfs, efs and nilfs are affected too.
 
 To make pullups easier we should probably do it in two steps, first
 this patch to msdosfs, pullup and then try changing bread.
 --
 Juergen Hannken-Illjes - hannken%eis.cs.tu-bs.de@localhost - TU Braunschweig =
 (Germany)
 
 On Mar 31, 2012, at 11:30 PM, Chuck Silvers wrote:
 
 > The following reply was made to PR kern/46282; it has been noted by =
 GNATS.
 >=20
 > From: Chuck Silvers <chuq%chuq.com@localhost>
 > To: gnats-bugs%NetBSD.org@localhost
 > Cc:=20
 > Subject: Re: kern/46282: 6.0_BETA crash: msdosfs_bmap -> pcbmap -> =
 bread ->
 > bio_doread
 > Date: Sat, 31 Mar 2012 14:25:26 -0700
 >=20
 > On Sat, Mar 31, 2012 at 07:55:04AM +0000, J. Hannken-Illjes wrote:
 >> Please try the attached patch. It replaces bread() with
 >> getblk() / VOP_STRATEGY() and returns an error if getblk() fails.
 >=20
 > I don't think that expanding bread() like this is the way to go.
 > several other file systems (nilfs, ntfs, maybe others) also have this =
 problem,
 > we don't want a bunch of copies of this all over the place.
 > it would be better to make bread() and bio_doread() handle this case.
 >=20
 > there's a wrinkle with that though.  currently bread() always returns
 > a buffer via bpp even if it returns an error, and the caller is =
 expected
 > to brelse() that buffer.  if getblk() returns NULL then we have no =
 buffer
 > for the caller to brelse().  the best thing to do would be to change =
 things
 > so that if bread() returns an error, bread() takes care of brelse()ing
 > the buffer (if any) instead of the caller doing it.  that means =
 changing
 > all ~140 callers, but it's a pretty straightforward change.
 >=20
 > -Chuck
 >=20