2015-11-02 06:48:35

by Kweh, Hock Leong

[permalink] [raw]
Subject: RE: [PATCH v9 1/1] efi: a misc char interface for user to update efi firmware

> -----Original Message-----
> From: Borislav Petkov [mailto:[email protected]]
> Sent: Sunday, November 01, 2015 6:30 PM
> >
> > Example method to load the capsule binary:
> > cat firmware.bin > /dev/efi_capsule_loader
>
> $ cat "some_dumb_file" > /dev/efi_capsule_loader
> Killed
>
> and in dmesg:
>
> [ 34.033982] efi_capsule_loader: efi_capsule_flush: capsule upload not
> complete

Hi Boris,

I have tested "cat /bin/ls > /dev/efi_capsule_loader" in my environment,
but I am not able to reproduce the issue. So, it is a bit hard for me to debug
the issue with my environment and may need your help on this.

By looking at your dmesg log, the above print out message seem that
someone has called the flush() after the write(2). In my environment, flush()
only being called in 2 places which are before write(2) and during close(2).
The dmesg log seems that your environment is running write(2) and flush() in
different threads and are parallel. Could you help me to double confirm this and it
would be good if you could told me when the flush() is exactly being called in
your environment. The info really help me on debugging.

Thanks & Regards,
Wilson

> [ 58.765683] ------------[ cut here ]------------
> [ 58.769349] WARNING: CPU: 5 PID: 3904 at
> drivers/firmware/efi/capsule.c:83 efi_capsule_supported+0x103/0x150()
> [ 58.775063] Modules linked in:
> [ 58.776474] CPU: 5 PID: 3904 Comm: cat Not tainted 4.3.0-rc7+ #3
> [ 58.779044] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.7.5-20140531_083030-gandalf 04/01/2014
> [ 58.783387] ffffffff81957aa0 ffff880079793d78 ffffffff812cb2ea
> 0000000000000000
> [ 58.786749] ffff880079793db0 ffffffff81055981 00010102464c457f
> 0000000000000000
> [ 58.790140] 0000000000401e3b 0000000000000001 ffff880078660704
> ffff880079793dc0
> [ 58.793353] Call Trace:
> [ 58.794343] [<ffffffff812cb2ea>] dump_stack+0x4e/0x84
> [ 58.796416] [<ffffffff81055981>] warn_slowpath_common+0x91/0xd0
> [ 58.798773] [<ffffffff81055a7a>] warn_slowpath_null+0x1a/0x20
> [ 58.800962] [<ffffffff8157ae93>] efi_capsule_supported+0x103/0x150
> [ 58.803292] [<ffffffff8157d559>] efi_capsule_write+0x269/0x390
> [ 58.805563] [<ffffffff81183ef8>] __vfs_write+0x28/0xe0
> [ 58.807591] [<ffffffff81183e9a>] ? __vfs_read+0xaa/0xe0
> [ 58.809612] [<ffffffff811847d5>] vfs_write+0xb5/0x1a0
> [ 58.811272] [<ffffffff811a33be>] ? __fget_light+0x6e/0x90
> [ 58.813073] [<ffffffff81185412>] SyS_write+0x52/0xc0
> [ 58.814720] [<ffffffff816cff5b>] entry_SYSCALL_64_fastpath+0x16/0x73
> [ 58.816665] ---[ end trace 94c0c141f9b0ec01 ]---
> [ 58.818179] BUG: unable to handle kernel NULL pointer dereference at
> (null)
> [ 58.820427] IP: [< (null)>] (null)
> [ 58.820630] PGD 79af8067 PUD 79781067 PMD 0
> [ 58.820630] Oops: 0010 [#1] PREEMPT SMP
> [ 58.820630] Modules linked in:
> [ 58.820630] CPU: 5 PID: 3904 Comm: cat Tainted: G W 4.3.0-rc7+ #3
> [ 58.820630] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.7.5-20140531_083030-gandalf 04/01/2014
> [ 58.820630] task: ffff8800771417c0 ti: ffff880079790000 task.ti:
> ffff880079790000
> [ 58.820630] RIP: 0010:[<0000000000000000>] [< (null)>] (null)
> [ 58.820630] RSP: 0018:ffff880079793dc8 EFLAGS: 00010282
> [ 58.820630] RAX: ffff88007a01b4e0 RBX: 00010102464c457f RCX:
> ffff880078660704
> [ 58.820630] RDX: ffff880079793dd8 RSI: 0000000000000001 RDI:
> ffff880079793dd0
> [ 58.820630] RBP: ffff880079793e08 R08: 0000000000000000 R09:
> 0000000000000000
> [ 58.820630] R10: 0000000000000000 R11: 0000000000000001 R12:
> 0000000000000000
> [ 58.820630] R13: 0000000000401e3b R14: 0000000000000001 R15:
> ffff880078660704
> [ 58.820630] FS: 00007ffff7fe1700(0000) GS:ffff88007c000000(0000)
> knlGS:0000000000000000
> [ 58.820630] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 58.820630] CR2: 0000000000000000 CR3: 000000007ab90000 CR4:
> 00000000000406e0
> [ 58.820630] Stack:
> [ 58.820630] ffffffff8157ae24 ffff88007a01b4e0 0000000000000002
> ffff880078660700
> [ 58.820630] ffff880077060000 0000000000001000 ffffea0001dc1800
> ffff880077060000
> [ 58.820630] ffff880079793e48 ffffffff8157d559 0000000000000402
> ffff8800799cbc00
> [ 58.820630] Call Trace:
> [ 58.820630] [<ffffffff8157ae24>] ? efi_capsule_supported+0x94/0x150
> [ 58.820630] [<ffffffff8157d559>] efi_capsule_write+0x269/0x390
> [ 58.820630] [<ffffffff81183ef8>] __vfs_write+0x28/0xe0
> [ 58.820630] [<ffffffff81183e9a>] ? __vfs_read+0xaa/0xe0
> [ 58.820630] [<ffffffff811847d5>] vfs_write+0xb5/0x1a0
> [ 58.820630] [<ffffffff811a33be>] ? __fget_light+0x6e/0x90
> [ 58.820630] [<ffffffff81185412>] SyS_write+0x52/0xc0
> [ 58.820630] [<ffffffff816cff5b>] entry_SYSCALL_64_fastpath+0x16/0x73
> [ 58.820630] Code: Bad RIP value.
> [ 58.820630] RIP [< (null)>] (null)
> [ 58.820630] RSP <ffff880079793dc8>
> [ 58.820630] CR2: 0000000000000000
> [ 58.876221] ---[ end trace 94c0c141f9b0ec02 ]---
>

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?


2015-11-03 19:59:40

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v9 1/1] efi: a misc char interface for user to update efi firmware

On Mon, Nov 02, 2015 at 06:47:29AM +0000, Kweh, Hock Leong wrote:
> By looking at your dmesg log, the above print out message seem that
> someone has called the flush() after the write(2). In my environment, flush()
> only being called in 2 places which are before write(2) and during close(2).
> The dmesg log seems that your environment is running write(2) and flush() in
> different threads and are parallel. Could you help me to double confirm this and it
> would be good if you could told me when the flush() is exactly being called in
> your environment. The info really help me on debugging.

I don't know what you mean: I simply do

cat /bin/ls > /dev/efi_capsule_loader

as root in an SMP kvm guest. And it explodes. Nothing special, just this
one command.

I guess you could try to reproduce it, here's how I start it:

qemu-system-x86_64
-enable-kvm
-gdb tcp::1234
-cpu Opteron_G5
-m 2048
-hda /home/boris/kvm/debian/sid-x86_64.img
-hdb /home/boris/kvm/swap.img
-boot menu=off,order=c
-localtime
-net nic,model=rtl8139
-net user,hostfwd=tcp::1235-:22
-usbdevice tablet
-kernel /home/boris/kernel/linux-2.6/arch/x86/boot/bzImage
-append "root=/dev/sda1 resume=/dev/sdb1 debug ignore_loglevel log_buf_len=16M earlyprintk=ttyS0,115200 console=ttyS0,115200 console=tty0"
-monitor pty
-virtfs local,path=/tmp,mount_tag=tmp,security_model=none
-serial file:/home/boris/kvm/test-x86_64-1235.log
-snapshot
-smp 8

HTH.

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.

2015-12-16 11:09:58

by Kweh, Hock Leong

[permalink] [raw]
Subject: RE: [PATCH v9 1/1] efi: a misc char interface for user to update efi firmware

> -----Original Message-----
> From: Borislav Petkov [mailto:[email protected]]
> Sent: Wednesday, November 04, 2015 4:00 AM
>
> On Mon, Nov 02, 2015 at 06:47:29AM +0000, Kweh, Hock Leong wrote:
> > By looking at your dmesg log, the above print out message seem that
> > someone has called the flush() after the write(2). In my environment,
> flush()
> > only being called in 2 places which are before write(2) and during close(2).
> > The dmesg log seems that your environment is running write(2) and flush()
> in
> > different threads and are parallel. Could you help me to double confirm this
> and it
> > would be good if you could told me when the flush() is exactly being called
> in
> > your environment. The info really help me on debugging.
>
> I don't know what you mean: I simply do
>
> cat /bin/ls > /dev/efi_capsule_loader
>
> as root in an SMP kvm guest. And it explodes. Nothing special, just this
> one command.
>
> I guess you could try to reproduce it, here's how I start it:
>
> qemu-system-x86_64
> -enable-kvm
> -gdb tcp::1234
> -cpu Opteron_G5
> -m 2048
> -hda /home/boris/kvm/debian/sid-x86_64.img
> -hdb /home/boris/kvm/swap.img
> -boot menu=off,order=c
> -localtime
> -net nic,model=rtl8139
> -net user,hostfwd=tcp::1235-:22
> -usbdevice tablet
> -kernel /home/boris/kernel/linux-2.6/arch/x86/boot/bzImage
> -append "root=/dev/sda1 resume=/dev/sdb1 debug ignore_loglevel
> log_buf_len=16M earlyprintk=ttyS0,115200 console=ttyS0,115200
> console=tty0"
> -monitor pty
> -virtfs local,path=/tmp,mount_tag=tmp,security_model=none
> -serial file:/home/boris/kvm/test-x86_64-1235.log
> -snapshot
> -smp 8
>
> HTH.
>
> --
> Regards/Gruss,
> Boris.

Hi Borislav,

Finally able to free up 25GB space to setup a QEMU VM with Debian v8.2.0
system and look into this issue. Located the NULL pointer happened at code line:

status = efi.query_capsule_caps(&capsule, 1, &max_size, reset);

which is inside function efi_capsule_supported(). This function call is initialized by
EFI Firmware run-time service table. So, I believe the QEMU do not emulate the
EFI Firmware run-time service API calls. This is why when come to this line it hit
the NULL pointer issue.

So, my conclusion is that this module is not able to be tested on QEMU environment.

Thanks & Regards,
Wilson

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2015-12-16 11:26:21

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v9 1/1] efi: a misc char interface for user to update efi firmware

On Wed, Dec 16, 2015 at 11:09:50AM +0000, Kweh, Hock Leong wrote:
> So, my conclusion is that this module is not able to be tested on QEMU
> environment.

That's not the point.

The module should better handle writing to the device file gracefully
and not explode. Regardless of whether it is running on an EFI system or
not.

efi_capsule_loader_init() simply loads the driver on *any* system,
even a !UEFI one. And when I write some garbage to the device file, it
explodes.

What it should do instead is check whether it is being loaded on en EFI
system and whether all it needs to function properly is initialized
already, like runtime services. If not, it should refuse to load.

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.

2015-12-17 01:59:34

by Kweh, Hock Leong

[permalink] [raw]
Subject: RE: [PATCH v9 1/1] efi: a misc char interface for user to update efi firmware

> -----Original Message-----
> From: Borislav Petkov [mailto:[email protected]]
> Sent: Wednesday, December 16, 2015 7:26 PM
> To: Kweh, Hock Leong
> Cc: Matt Fleming; Greg Kroah-Hartman; Ong, Boon Leong; LKML; linux-
> [email protected]; Sam Protsenko; Peter Jones; Andy Lutomirski; Roy
> Franz; James Bottomley; Linux FS Devel; Anvin, H Peter; 'Matt Fleming'
> Subject: Re: [PATCH v9 1/1] efi: a misc char interface for user to update efi
> firmware
>
> On Wed, Dec 16, 2015 at 11:09:50AM +0000, Kweh, Hock Leong wrote:
> > So, my conclusion is that this module is not able to be tested on QEMU
> > environment.
>
> That's not the point.
>
> The module should better handle writing to the device file gracefully
> and not explode. Regardless of whether it is running on an EFI system or
> not.
>
> efi_capsule_loader_init() simply loads the driver on *any* system,
> even a !UEFI one. And when I write some garbage to the device file, it
> explodes.
>
> What it should do instead is check whether it is being loaded on en EFI
> system and whether all it needs to function properly is initialized
> already, like runtime services. If not, it should refuse to load.
>
> --
> Regards/Gruss,
> Boris.

Hi Borislav,

I catch your point now. I will fix that in v10 patch.

Thanks & Regards,
Wilson

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?