Messages in this thread | | | Date | Fri, 17 Oct 2003 16:55:43 -0700 | From | Andrew Morton <> | Subject | Re: [RFC] prevent "dd if=/dev/mem" crash |
| |
Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: > > On Friday 17 October 2003 4:50 pm, Andrew Morton wrote: > > Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: > > > > > > Old behavior: > > > > > > # dd if=/dev/mem of=/dev/null > > > <unrecoverable machine check> > > > > I recently fixed this for ia32 by changing copy_to_user() to not oops if > > the source address generated a fault. Similarly copy_from_user() returns > > an error if the destination generates a fault. > > > > In other words: drivers/char/mem.c requires that the architecture's > > copy_*_user() functions correctly handle faults on either the source or > > dest of the copy. > > If we really believe copy_*_user() must correctly handle *all* faults, > isn't the "p >= __pa(high_memory)" test superfluous?
This code was conceived before my time and I don't recall seeing much discussion, so this is all guesswork..
I'd say that the high_memory test _is_ superfluous and that if anyone cared, we would remove it and establish a temporary pte against the address if it was outside the direct-mapped area. But nobody cares enough to have done anything about it.
> I don't know how ia32 handles a read to non-existent physical memory. > Are you saying that copy_*_user() can deal with that just like it does > a garden-variety TLB fault?
I don't know, and I suspect it depends on the off-CPU hardware implementation anyway. But the access will either generate a fault or it won't and in either case we're OK, yes?
> On ia64, a read to non-existent physical memory causes the processor > to time out and take a machine check. I'm not sure it's even possible > to recover from that.
ick. That would be very poor form. What about things like probing for memory, device hot-unplug, memory hot unplug etc?
Still, the code you have is quite reasonable. But please structure it thusly:
#include <asm/io.h> /* valid_phys_addr_range */
#ifndef ARCH_HAS_VALID_PHYS_ADDR_RANGE static inline int valid_phys_addr_range(unsigned long addr, size_t *count) { unsigned long end_mem;
end_mem = __pa(high_memory); if (addr >= end_mem) return 0;
if (*count > end_mem - addr) *count = end_mem - addr;
return 1; } #endif
or whatever. It's a bit more conventional this way and allows other architectures to do appropriate things.
As for return values: if the requested read or write starts at a not-present address it should probably return -EFAULT. This is what ia32 will do. Arguably this is indistinguishable from a bad address on the userspace side and we should return -EINVAL but whatever.
If the request starts at a valid phys address but covers a not-present address it should return a short read or write (returns something less than `count').
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |