Re: Re: toshiba_acpi 0.18

[John Belmonte <john-wanGne27zNesTnJN9+BGXg@public.gmane.org>]

Sergey Vlasov wrote:
> On Sun, 14 Mar 2004 01:02:58 -0500 John Belmonte wrote:
> 
>> static int
>>-dispatch_write(struct file* file, const char* buffer, unsigned long count,
>>-	ProcItem* item)
>>+dispatch_write(struct file* file, __user const char* buffer,
>>+	unsigned long count, ProcItem* item)
>> {
>>-	return item->write_func(buffer, count);
>>+	int result;
>>+	char* tmp_buffer;
>>+
>>+	/* Arg buffer points to userspace memory, which can't be accessed
>>+	 * directly.  Since we're making a copy, zero-terminate the
>>+	 * destination so that sscanf can be used on it safely.
>>+	 */
>>+	tmp_buffer = kmalloc(count + 1, GFP_KERNEL);
>>+	if (copy_from_user(tmp_buffer, buffer, count)) {
>>+		result = -EFAULT;
>>+	}
>>+	else {
>>+		tmp_buffer[count] = 0;
>>+		result = item->write_func(tmp_buffer, count);
>>+	}
>>+	kfree(tmp_buffer);
>>+	return result;
>> }
> 
> 
> This is still not enough.  count comes from userspace and can be
> arbitrarily large, and this function does not even check the return
> value from kmalloc()...

The "count" arg is passed by value, so there is no issue.

You are right about not checking the kmalloc result for an error.  I'll 
fix that, but it's not enough to warrant a new release on its own.  The 
code before the patch had the same problem, so this is not a newly 
created bug, and not related to the problem being addressed.

-John


-- 
http:// if  ile.org/


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click