Re: [PATCH] bio_uncopy_user mem leak

[Greg Afinogenov <antisthenes@inbox.ru>]

On Thu, 2004-08-19 at 22:19, Con Kolivas wrote:
> Kurt Garloff wrote:
> > Hi Chris,
> > 
> > On Thu, Aug 19, 2004 at 09:51:34AM -0400, Chris Mason wrote:
> > 
> >>On Thu, 2004-08-19 at 07:07, Con Kolivas wrote:
> >>
> >>>Ok I just tested this patch discretely and indeed the memory leak goes 
> >>>away but it still produces coasters so something is still amuck. Just as 
> >>>a data point; burning DVDs and data cds is ok. Burning audio *and 
> >>>videocds* is not.
> >>
> >>It might be the cold medicine talking, but I think we need something
> >>like this.  gcc tested it for me, beyond that I make no promises....
> >>
> >>--- l/fs/bio.c.1	2004-08-19 09:36:13.596858736 -0400
> >>+++ l/fs/bio.c	2004-08-19 09:47:46.392537784 -0400
> >>@@ -454,6 +454,7 @@
> >> 	 */
> >> 	if (!ret) {
> >> 		if (!write_to_vm) {
> >>+			unsigned long p = uaddr;
> >> 			bio->bi_rw |= (1 << BIO_RW);
> >> 			/*
> >> 	 		 * for a write, copy in data to kernel pages
> >>@@ -462,8 +463,9 @@
> >> 			bio_for_each_segment(bvec, bio, i) {
> >> 				char *addr = page_address(bvec->bv_page);
> >> 
> >>-				if (copy_from_user(addr, (char *) uaddr, bvec->bv_len))
> >>+				if (copy_from_user(addr, (char *) p, bvec->bv_len))
> >> 					goto cleanup;
> >>+				p += bvec->bv_len;
> >> 			}
> >> 		}
> >> 
> > 
> > 
> > Hmm, that patch would make a lot of sense to me.
> > 
> > It matches the problem description; burning data CDs, we don't
> > use bounce buffers, so that does not use this code path. Here,
> > it looks like we copied the same userspace page again and again
> > into a multisegment BIO. Ouch!
> > 
> > Not yet tested either :-(
> 
> Ok looks like your cold medicine is working well for you ;-). This patch 
> on top of the other patch has the memory freeing _and_ burns good cds. 
> Well done. I only tested with a video cd. Can someone confirm audio cd 
> (although it seems obvious it would help both).
> 
> Andrew did you threaten to make a 2.6.8.2 since 2.6.8{,.1} cannot safely 
> burn an audio cd?
> 
> Cheers,
> Con

Yup, it works fine with audio CDs too.  Great!