* Re: [PATCH/2.4]: do_write_mem() return value check [not found] <200403231707.14088.blaisorblade_work@yahoo.it> @ 2004-03-25 20:52 ` Marcelo Tosatti 0 siblings, 0 replies; 2+ messages in thread From: Marcelo Tosatti @ 2004-03-25 20:52 UTC (permalink / raw) To: BlaisorBlade; +Cc: linux-kernel, akpm 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). > > - remove unused `file *' arg from do_write_mem() > > - Add checking for copy_from_user() failures in do_write_mem() > > (Note: /dev/kmem can be written to only by root, so this *cannot* have > security implications) > > - Return correct value from kmem writes() when a fault is encountered. A > write()-style syscall's return values are: > > 0 when nothing was written and there was no error (someone tried to > write zero bytes) > > >0: the number of bytes copied, whether or not there was an error. > Userspace detects errors by noting that the write() return value is less > than was requested. > > <0: there was an error and no bytes were copied > > TODO: Do the same changes for read_mem() and read_kmem(). The code is more > messy so I must create do_read_mem() to avoid clumsy counting; I will post the > patch first for 2.6. Hi Paolo, This is nice -- although I see it as a 2.6 cleanup only. Do you actually have any practical problem caused by the current broken "always return -EFAULT" ? Otherwise leave it for 2.6. ^ permalink raw reply [flat|nested] 2+ messages in thread
[parent not found: <20040326033339.GD1997@logos.cnet>]
* Re: [PATCH/2.4]: do_write_mem() return value check [not found] <20040326033339.GD1997@logos.cnet> @ 2004-03-26 18:16 ` BlaisorBlade 0 siblings, 0 replies; 2+ messages in thread From: BlaisorBlade @ 2004-03-26 18:16 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: linux-kernel 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 ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2004-03-27 4:40 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <200403231707.14088.blaisorblade_work@yahoo.it>
2004-03-25 20:52 ` [PATCH/2.4]: do_write_mem() return value check Marcelo Tosatti
[not found] <20040326033339.GD1997@logos.cnet>
2004-03-26 18:16 ` BlaisorBlade
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox