• Vegard Nossum's avatar
    On 2009/6/17 Ingo Molnar <mingo@elte.hu> reported: · 95096a22
    Vegard Nossum authored
    >
    > btw., here's an old friend of a warning:
    >
    > async_continuing @ 1 after 0 usec
    > WARNING: kmemcheck: Caught 8-bit read from freed memory (f5f33004)
    > 0040f3f57400686f74706c756700000000000000000000000000000000000000
    >  i i i i f f f f f f f f f f f f f f f f f f f f f f f f f f f f
    >          ^
    >
    > Pid: 1, comm: swapper Not tainted (2.6.30-tip-04303-g5ada65e-dirty #767) P4DC6
    > EIP: 0060:[<c1248df4>] EFLAGS: 00010246 CPU: 0
    > EIP is at exact_copy_from_user+0x64/0x130
    > EAX: 00000000 EBX: 00000001 ECX: 000000f5 EDX: 000000f5
    > ESI: f5fdeffb EDI: f5f33004 EBP: f6c48ee8 ESP: c29598cc
    >  DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
    > CR0: 8005003b CR2: f6c20044 CR3: 0294d000 CR4: 000006d0
    > DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
    > DR6: ffff4ff0 DR7: 00000400
    >  [<c124916a>] copy_mount_options+0xba/0x1c0
    >  [<c124dc0a>] sys_mount+0x1a/0x170
    >  [<c263c937>] do_mount_root+0x27/0xe0
    >  [<c263ca33>] mount_block_root+0x43/0x140
    >  [<c263cc02>] mount_root+0xd2/0x160
    >  [<c263ce49>] prepare_namespace+0x1b9/0x380
    >  [<c263c4c8>] kernel_init+0xb8/0x110
    >  [<c103ab13>] kernel_thread_helper+0x7/0x14
    >  [<ffffffff>] 0xffffffff
    > EXT3-fs: INFO: recovery required on readonly filesystem.
    > EXT3-fs: write access will be enabled during recovery.
    
    sys_mount() reads/copies a whole page for its "type" parameter.  When
    do_mount_root() passes a kernel address that points to an object which is
    smaller than a whole page, copy_mount_options() will happily go past this
    memory object, possibly dereferencing "wild" pointers that could be in any
    state (hence the kmemcheck warning, which shows that parts of the next
    page are not even allocated).
    
    (The likelihood of something going wrong here is pretty low -- first of
    all this only applies to kernel calls to sys_mount(), which are mostly
    found in the boot code.  Secondly, I guess if the page was not mapped,
    exact_copy_from_user() _would_ in fact handle it correctly because of its
    access_ok(), etc.  checks.)
    
    But it is much nicer to avoid the dubious reads altogether, by stopping as
    soon as we find a NUL byte.  Is there a good reason why we can't do
    something like this, using the already existing strndup_from_user()?
    
    [akpm@linux-foundation.org: make copy_mount_string() static]
    Reported-by: default avatarIngo Molnar <mingo@elte.hu>
    Signed-off-by: default avatarVegard Nossum <vegard.nossum@gmail.com>
    Cc: Al Viro <viro@zeniv.linux.org.uk>
    Cc: Pekka Enberg <penberg@cs.helsinki.fi>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    95096a22
namespace.c 57.2 KB