Messages in this thread | | | Subject | Re: [patch] fix race in __block_prepare_write (again) | From | Anton Altaparmakov <> | Date | Thu, 21 Apr 2005 08:10:01 +0100 |
| |
And one more thing...
On Thu, 2005-04-21 at 08:01 +0100, Anton Altaparmakov wrote: > On Thu, 21 Apr 2005, Nick Piggin wrote: > > ... I somehow didn't send it to Andrew last time. > > > > Fix a race where __block_prepare_write can leak out an in-flight > > read against a bh if get_block returns an error. This can lead to > > the page becoming unlocked while the buffer is locked and the read > > still in flight. __mpage_writepage BUGs on this condition. > [snip] > > --- linux-2.6.orig/fs/buffer.c 2005-04-21 11:55:17.549614278 > +1000 > > +++ linux-2.6/fs/buffer.c 2005-04-21 15:55:41.483826075 +1000 > > @@ -1988,6 +1988,7 @@ > > *wait_bh++=bh; > > } > > } > > +out: > > /* > > * If we issued read requests - let them complete. > > */ > > @@ -1996,8 +1997,9 @@ > > if (!buffer_uptodate(*wait_bh)) > > return -EIO;
This return is now wrong after your patch. It should be "err = -EIO;" otherwise you do not zero newly allocated blocks and thus risk exposing stale data on buffer i/o errors.
> > } > > - return 0; > > -out: > > + if (!err) > > + return err; > > + > > /* > > * Zero out any newly allocated blocks to avoid exposing stale > > * data. If BH_New is set, we know that the block was newly > > Any reason why you left the goto out? It would be IMO much cleaner to > remove the label "out" altogether and replace the single "goto out" with a > "break" (which is fine since the goto happens inside the for loop > immediately after which you place the label.)
Best regards,
Anton -- Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @) Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |