Messages in this thread | | | Date | Fri, 20 Aug 2004 13:19:37 +1000 | From | Con Kolivas <> | Subject | Re: [PATCH] bio_uncopy_user mem leak |
| |
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 [unhandled content-type:application/pgp-signature] | |