linux-kernel - Re: shmem_fill_super(): WARNING: kmemcheck: Caught 32-bit read from uninitialized memory

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <19f34abd0909201035t3157948amee532a3a5e96dbab@mail.gmail.com>
Date:	Sun, 20 Sep 2009 19:35:03 +0200
From:	Vegard Nossum <vegard.nossum@...il.com>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Pekka Enberg <penberg@...helsinki.fi>,
	linux-kernel@...r.kernel.org, Eric Paris <eparis@...hat.com>
Subject: Re: shmem_fill_super(): WARNING: kmemcheck: Caught 32-bit read from 
	uninitialized memory

2009/9/20 Ingo Molnar <mingo@...e.hu>:
>
> here's another one:
>
> [    0.337000] Total of 1 processors activated (3088.38 BogoMIPS).
> [    0.352000] CPU0 attaching NULL sched-domain.
> [    0.360000] WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (9f8020fc)
> [    0.361000] a44240820000000041f6998100000000000000000000000000000000ff030000
> [    0.368000]  i i i i i i i i i i i i i i i i u u u u i i i i i i i i i i u u
> [    0.375000]                                                          ^
> [    0.376000]
> [    0.377000] Pid: 9, comm: khelper Not tainted (2.6.31-tip #206) P4DC6
> [    0.378000] EIP: 0060:[<810a3a95>] EFLAGS: 00010246 CPU: 0
> [    0.379000] EIP is at shmem_fill_super+0xb5/0x120
> [    0.380000] EAX: 00000000 EBX: 9f845400 ECX: 824042a4 EDX: 8199f641
> [    0.381000] ESI: 9f8020c0 EDI: 9f845400 EBP: 9f81af68 ESP: 81cd6eec

>
> Is this one known too?
>
>        Ingo
>

Thanks for the report.

AFAICT it's this line of mm/shmem.c:

2356         inode = shmem_get_inode(sb, S_IFDIR | sbinfo->mode, 0,
VM_NORESERVE     );

and the loading of sbinfo->mode. It fits with the offset 0x3c(%esi) ==
the address reported by kmemcheck and the offset of ->mode:

(gdb) p &((struct shmem_sb_info *) 0).mode
$1 = (mode_t *) 0x3c

Looking for the definition of mode_t, it seems to be defined in x86
sources as unsigned short:

arch/x86/include/asm/posix_types_32.h:11:typedef unsigned short __kernel_mode_t;
include/linux/types.h:typedef __kernel_mode_t           mode_t;

And the load was clearly 32-bit (kmemcheck said so) and in my assembly
dump it is also so.

As I said before, I really don't like the solution of sprinkling the
kmemcheck annotations all over the place to cover up field padding
inside structs, not in the least because they confuse more than they
help, and they are not maintainable -- when somebody changes the
struct definitions, anything may happen to the field layout, and then
the annotation may have to change too. And it's not exactly obvious.

I still vote for patching gcc as the long-term solution. There is
-fmudflap, there is -fstack-protector, why not a -fsacred-padding? Of
course it has to be implemented too...


Vegard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/