Messages in this thread | | | From | (Linus Torvalds) | Subject | Re: error in msync(2) or am I missing something? | Date | 4 Sep 1998 17:30:05 GMT |
| |
In article <Pine.LNX.4.02.9809041447310.5239-100000@einstein.london.sco.com>, Tigran Aivazian <tigran@sco.COM> wrote: >Hello guys, > >In the msync(2) (source mm/filemap.c/sys_msync()) function there is a piece of >code: > >len = (len + ~PAGE_MASK) & PAGE_MASK; >end = start + len; >if (end < start) > goto out; >if (flags & ~(MS_ASYNC | MS_INVALIDATE | MS_SYNC)) > goto out; >error = 0; >if (end == start) > goto out; > >Now, len is size_t (unsigned int) so it can hardly ever become negative. So, >can't one simply rewrite the code as:
[ removed so that people don't get ideas ;-]
It's not that "len" can be negative, it's about wraparound. The "end < start" bit is essentially testing the carry from the (unsigned) addition one line above it. In fact, a good compiler will notice that, and actually do the addition and the compare with a "add + jc" combination.
(There is no notion of carry in normal C, which is why it has to be done this way if you want to be portable).
So you have three cases: - wrapping addresses: bad, and should return an error - an empty area (ok - return zero without doing anything) - an actual potentially valid area (do some real work)
Note that address wrap-around is a _really_ common security issue in operating systems. Linux has had its share, but probably fewer than most because I and others have been so aware of it. If you don't check for wrap-around, it is often fairly easy to fool a system into believing you are giving it a good address range, even when you are not.
For example, assume that you have an OS that wants to restrict all user pointers to the low virtual memory area (fairly common), and you're given a range. The _wrong_ way to do this is to have something like
if (start + len > limit) error
because that leaves you open to a malicious user that passes in an address inside your protected region, but makes sure that the length is big enough that the addition will wrap.
So you should do something like
if (start + len < start || start + len > limit) error
where the first part guarantees that it doesn't wrap, and the second part guarantees that all of the area is below the actual limit (and again, a good compiler will actually turn the above into something like
/* start in %eax, len in %edx */ add %eax,%edx jc bad cmpl limit,%eax ja bad
so it's actually only one extra check against carry in addition to the limit check.
Look into asm-i386/uaccess.h for alternative ways of doing ranges right by explicitly using the carry bit. I don't know if it's any better, but it's different, and as such could be instructive. Linus
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/faq.html
| |