| 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
| ||
|
Message-ID: <20111207142907.GC4510@home.goodmis.org>
Date: Wed, 7 Dec 2011 09:29:07 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [3.2-rc4 x86] (Compiler bug?) Uninitialized variable in
ioapic_read_entry()
On Wed, Dec 07, 2011 at 10:20:43PM +0900, Tetsuo Handa wrote:
> Regarding commit e57253a8 "x86, ioapic: Restore the mask bit correctly in
> eoi_ioapic_irq()", I got below warning:
>
> arch/x86/kernel/apic/io_apic.c: In function 'ioapic_read_entry':
> arch/x86/kernel/apic/io_apic.c:385: warning: 'eu' is used uninitialized in this function
>
> 369: union entry_union {
> 370: struct { u32 w1, w2; };
> 371: struct IO_APIC_route_entry entry;
> 372: };
> 373:
> 374: static struct IO_APIC_route_entry __ioapic_read_entry(int apic, int pin)
> 375: {
> 376: union entry_union eu;
> 377:
> 378: eu.w1 = io_apic_read(apic, 0x10 + 2 * pin);
> 379: eu.w2 = io_apic_read(apic, 0x11 + 2 * pin);
> 380: return eu.entry;
> 381: }
> 382:
> 383: static struct IO_APIC_route_entry ioapic_read_entry(int apic, int pin)
> 384: {
> 385: union entry_union eu;
> 386: unsigned long flags;
> 387: raw_spin_lock_irqsave(&ioapic_lock, flags);
> 388: eu.entry = __ioapic_read_entry(apic, pin);
> 389: raw_spin_unlock_irqrestore(&ioapic_lock, flags);
> 390: return eu.entry;
> 391: }
The above code looks fine. The only real differences is that the
"packed" attribute is not used by the above, but that really shouldn't
matter because gcc should always pack two u32 words together.
>
> I compared using several gcc versions.
> It turned out that only gcc 4.4 shows this warning.
>
[snip]
>
> It turned out that gcc 4.3 and 4.5 do not inline __ioapic_read_entry() whereas
> gcc 4.4 inlines __ioapic_read_entry().
>
> --- gcc (Debian 4.3.2-1.1) 4.3.2 on Debian Lenny ---
Lets compare:
>
> 00000040 <__ioapic_read_entry>:
> 40: 56 push %esi
> 41: 53 push %ebx
> 42: 8d 58 05 lea 0x5(%eax),%ebx
> 45: 8d 04 80 lea (%eax,%eax,4),%eax
> 48: 8b 0c c5 ac 08 00 00 mov 0x8ac(,%eax,8),%ecx
> 4f: c1 e3 0c shl $0xc,%ebx
> 52: 8d 74 12 10 lea 0x10(%edx,%edx,1),%esi
> 56: 81 e1 ff 0f 00 00 and $0xfff,%ecx
> 5c: 03 0d 00 00 00 00 add 0x0,%ecx
> 62: 29 d9 sub %ebx,%ecx
> 64: 89 31 mov %esi,(%ecx)
> 66: 83 c1 10 add $0x10,%ecx
> 69: 8b 09 mov (%ecx),%ecx
> 6b: 8b 04 c5 ac 08 00 00 mov 0x8ac(,%eax,8),%eax
> 72: 8d 54 12 11 lea 0x11(%edx,%edx,1),%edx
> 76: 25 ff 0f 00 00 and $0xfff,%eax
> 7b: 03 05 00 00 00 00 add 0x0,%eax
> 81: 29 d8 sub %ebx,%eax
> 83: 89 10 mov %edx,(%eax)
> 85: 83 c0 10 add $0x10,%eax
> 88: 8b 10 mov (%eax),%edx
> 8a: 89 c8 mov %ecx,%eax
> 8c: 5b pop %ebx
> 8d: 5e pop %esi
> 8e: c3 ret
> 8f: 90 nop
>
[ snip ]
>
> --- gcc (Ubuntu 4.4.3-4ubuntu5) 4.4.3 on Ubuntu 10.04 ---
>
> 00001d60 <ioapic_read_entry>:
> 1d60: 83 ec 0c sub $0xc,%esp
> 1d63: 89 1c 24 mov %ebx,(%esp)
> 1d66: 89 c3 mov %eax,%ebx
> 1d68: b8 4e 08 00 00 mov $0x84e,%eax
> 1d6d: 89 74 24 04 mov %esi,0x4(%esp)
> 1d71: 89 7c 24 08 mov %edi,0x8(%esp)
> 1d75: 89 d7 mov %edx,%edi
> 1d77: e8 fc ff ff ff call 1d78 <ioapic_read_entry+0x18>
to this:
> 1d7c: 8d 53 05 lea 0x5(%ebx),%edx
> 1d7f: 8d 1c 9b lea (%ebx,%ebx,4),%ebx
> 1d82: 8b 0c dd ac 08 00 00 mov 0x8ac(,%ebx,8),%ecx
> 1d89: c1 e2 0c shl $0xc,%edx
> 1d8c: 8d 74 3f 10 lea 0x10(%edi,%edi,1),%esi
> 1d90: 81 e1 ff 0f 00 00 and $0xfff,%ecx
> 1d96: 03 0d 00 00 00 00 add 0x0,%ecx
> 1d9c: 29 d1 sub %edx,%ecx
> 1d9e: 89 31 mov %esi,(%ecx)
> 1da0: 8b 71 10 mov 0x10(%ecx),%esi
almost identical, where the former does:
add $0x10,%eax
mov (%eax), %edx
the latter does:
mov 0x10(%ecx), %ebx
which is basically the same.
> 1da3: 8b 0c dd ac 08 00 00 mov 0x8ac(,%ebx,8),%ecx
> 1daa: 8d 7c 3f 11 lea 0x11(%edi,%edi,1),%edi
> 1dae: 81 e1 ff 0f 00 00 and $0xfff,%ecx
> 1db4: 03 0d 00 00 00 00 add 0x0,%ecx
> 1dba: 29 d1 sub %edx,%ecx
> 1dbc: 89 39 mov %edi,(%ecx)
> 1dbe: 8b 59 10 mov 0x10(%ecx),%ebx
Again the shortcut is done.
> 1dc1: 89 c2 mov %eax,%edx
> 1dc3: b8 4e 08 00 00 mov $0x84e,%eax
> 1dc8: e8 fc ff ff ff call 1dc9 <ioapic_read_entry+0x69>
> 1dcd: 89 f0 mov %esi,%eax
> 1dcf: 89 da mov %ebx,%edx
> 1dd1: 8b 1c 24 mov (%esp),%ebx
> 1dd4: 8b 74 24 04 mov 0x4(%esp),%esi
> 1dd8: 8b 7c 24 08 mov 0x8(%esp),%edi
> 1ddc: 83 c4 0c add $0xc,%esp
> 1ddf: c3 ret
>
> --- gcc (GCC) 4.5.1 20100924 (Red Hat 4.5.1-4) on Fedora 14 ---
>
> 00000020 <__ioapic_read_entry>:
> 20: 56 push %esi
> 21: 8d 48 05 lea 0x5(%eax),%ecx
> 24: 8d 04 80 lea (%eax,%eax,4),%eax
> 27: 53 push %ebx
> 28: 8d 1c c5 60 08 00 00 lea 0x860(,%eax,8),%ebx
> 2f: 8b 43 0c mov 0xc(%ebx),%eax
I'm not sure why gcc did this differently.
The previous ones did:
mov 0x8ac(,%ebx,8),%ecx
Which is basically to load the contents at %ebx*8 + 0x8ac into %ecx
but the above does it in two steps:
%ebx = 0x860 + %eax * 8
load %eax with contest at %ebx + 0xc
Which is not the same address. Did you compile with the same options?
Maybe I just did my math wrong (I just woke up ;)
> 32: c1 e1 0c shl $0xc,%ecx
> 35: 8d 74 12 10 lea 0x10(%edx,%edx,1),%esi
> 39: 25 ff 0f 00 00 and $0xfff,%eax
> 3e: 03 05 00 00 00 00 add 0x0,%eax
> 44: 29 c8 sub %ecx,%eax
> 46: 89 30 mov %esi,(%eax)
> 48: 8b 40 10 mov 0x10(%eax),%eax
> 4b: 8d 74 12 11 lea 0x11(%edx,%edx,1),%esi
> 4f: 8b 53 0c mov 0xc(%ebx),%edx
> 52: 81 e2 ff 0f 00 00 and $0xfff,%edx
> 58: 03 15 00 00 00 00 add 0x0,%edx
> 5e: 29 ca sub %ecx,%edx
> 60: 89 32 mov %esi,(%edx)
> 62: 8b 52 10 mov 0x10(%edx),%edx
Here it does the shortcut version too.
> 65: 5b pop %ebx
> 66: 5e pop %esi
> 67: c3 ret
> 68: 90 nop
> 69: 8d b4 26 00 00 00 00 lea 0x0(%esi,%eiz,1),%esi
>
> 00000200 <ioapic_read_entry>:
> 200: 83 ec 0c sub $0xc,%esp
> 203: 89 1c 24 mov %ebx,(%esp)
> 206: 89 c3 mov %eax,%ebx
> 208: b8 62 12 00 00 mov $0x1262,%eax
> 20d: 89 74 24 04 mov %esi,0x4(%esp)
> 211: 89 d6 mov %edx,%esi
> 213: 89 7c 24 08 mov %edi,0x8(%esp)
> 217: e8 fc ff ff ff call 218 <ioapic_read_entry+0x18>
> 21c: 89 f2 mov %esi,%edx
> 21e: 89 c7 mov %eax,%edi
> 220: 89 d8 mov %ebx,%eax
> 222: e8 f9 fd ff ff call 20 <__ioapic_read_entry>
> 227: 89 c3 mov %eax,%ebx
> 229: 89 d6 mov %edx,%esi
> 22b: b8 62 12 00 00 mov $0x1262,%eax
> 230: 89 fa mov %edi,%edx
> 232: e8 fc ff ff ff call 233 <ioapic_read_entry+0x33>
> 237: 89 d8 mov %ebx,%eax
> 239: 89 f2 mov %esi,%edx
> 23b: 8b 1c 24 mov (%esp),%ebx
> 23e: 8b 74 24 04 mov 0x4(%esp),%esi
> 242: 8b 7c 24 08 mov 0x8(%esp),%edi
> 246: 83 c4 0c add $0xc,%esp
> 249: c3 ret
> 24a: 8d b6 00 00 00 00 lea 0x0(%esi),%esi
>
> Also, it turned out that this warning does not apprar when compiled with
> CONFIG_M586MMX and earlier. This warning appears when compiled with CONFIG_M686
> and later.
>
> I don't know whether this warning is a false positive or not.
> Can somebody who understand assembly code check this?
Don's sweat it. It looks like a false positive. Gcc just got a little confused by
the union there.
-- Steve
>
--
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/
Powered by blists - more mailing lists