Re: [PATCH] bio_uncopy_user mem leak

[Con Kolivas]

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

Attachment: signature.asc
Description: OpenPGP digital signature