Re: [PATCH/2.4]: do_write_mem() return value check

From: BlaisorBlade
Date: Fri Mar 26 2004 - 23:43:58 EST

Alle 04:33, venerdì 26 marzo 2004, Marcelo Tosatti ha scritto:
> On Wed, Mar 24, 2004 at 07:59:01PM +0100, BlaisorBlade wrote:
> > From: Andrew Morton, and me (I did a first fix for 2.6 and sent to him,
> > he checked everything and committed it and I changed the trivial bits for
> > 2.4).

> Do you actually have any practical problem caused by the current broken
> "always return -EFAULT" ?

It is not only that, there can be also data corruption:

- The "always return -EFAULT" one is a little bug, so you can discard the
first and third hunk which fix that; anyway, it was Andrew Morton who noticed
that, so I've no idea (I agree with you but I'm a kernel newbie). Note I
posted a 2nd version of the patch which fixes a bug in the third hunk (see at
the end).

- BUT in the hunk below (the 2nd in the patch), without the patch, the var
"wrote" can be == -EFAULT and WE CAN DO
p += -EFAULT,
buf += -EFAULT;

so that we can read, afterwards, some bytes of junk from buf - EFAULT and put
them into p - EFAULT, *corrupting memory* !!!!

> > - Add checking for copy_from_user() failures in do_write_mem()

if (p < (unsigned long) high_memory) {
+ ssize_t written;
wrote = count;
if (count > (unsigned long) high_memory - p)
wrote = (unsigned long) high_memory - p;

- wrote = do_write_mem(file, (void*)p, p, buf, wrote, ppos);
+ written = do_write_mem((void*)p, p, buf, wrote, ppos);
+ if (written != wrote)
+ return written;
+ wrote = written;

p += wrote;
buf += -EFAULT;

Fix in the second version of the patch, for the 3rd hunk:
- Fix this line:
+ unwritten = copy_from_user(kbuf, buf, len);
+ if (unwritten != len) {
[... error handling]
to this:
+ unwritten = copy_from_user(kbuf, buf, len);
+ if (unwritten != 0) {
[...error handling]
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at