Re: [PATCH] bio_uncopy_user mem leak

[Con Kolivas <kernel@kolivas.org>]

[-- Attachment #1: Type: text/plain, Size: 1851 bytes --]

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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]