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  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]
Date:	Mon, 10 Nov 2014 11:31:06 -0800
From:	Andy Lutomirski <>
To:	"Kweh, Hock Leong" <>
Cc:	Sam Protsenko <>,
	"" <>,
	Greg KH <>,
	"Fleming, Matt" <>,
	"Ong, Boon Leong" <>,
	Ming Lei <>,
	"" <>
Subject: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface

On Mon, Nov 10, 2014 at 12:31 AM, Kweh, Hock Leong
<> wrote:
>> -----Original Message-----
>> From: Andy Lutomirski []
>> > #!/bin/sh
>> >
>> > old=$(cat
>> > /sys/devices/platform/efi_capsule_user_helper/capsule_loaded)
>> >
>> > for arg in "$@"
>> > do
>> >         if [ -f $arg ]
>> >         then
>> >                 echo 1 > /sys/class/firmware/efi-capsule-file/loading
>> >                 cat $arg > /sys/class/firmware/efi-capsule-file/data
>> >                 echo 0 > /sys/class/firmware/efi-capsule-file/loading
>> I think you have a race.  Try putting msleep(1000) after the
>> request_firmware_nowait call, and I bet this will fail on the second try.
> Sorry for the late response. I don't really catch the race condition that
> you are referring? Are you trying to tell that the user script could run faster
> before the previous callback function actually end? Will such scenario happen?
> In the callback function, after the request_firmware_nowait(), I don't have
> any codes will delay the callback function to end. Besides, there is a mutex_lock
> protecting the request_firmware_nowait() calling. Won't that take care of the
> issue?

In callbackfn_efi_capsule, you call request_firmware_nowait.  When
that callback is invoked, I think that the
/sys/class/firmware/efi-capsule-file directory doesn't exist at all.
If the callback takes longer than it takes your script to make it
through a full iteration, then it will try uploading the second
capsule before the firmware class directory is there, so it will fail.

But I just realized that your script has a loop below to handle that.
It's this:

                 oldtime=$(date +%S)
                 oldtime=$(((time + 2) % 60))
                 until [ -f /sys/class/firmware/efi-capsule-file/loading ]
                         newtime=$(date +%S)
                         if [ $newtime -eq $oldtime ]

Aside from the fact that this loop itself is racy (it may loop forever
if something goes wrong in the kernel, since $newtime -eq $oldtime may
never happen), it should help, if you're lucky.  But there's another

>> I think that firmware_class doesn't call the callback until after loading is closed
>> for the second time.  If so, then this is racy.  Try inserting msleep(1000) at the
>> beginning of your callback and uploading a capsule that should load
>> successfully -- this will report failure, but a future upload may get very
>> confused. Also, what does the firmware class do when simultaneous
>> uploads of the same file with different contents are in flight?  Is that possible?
> Sorry again, I can't really catch you on this race condition statement. Are you
> trying to tell if user is doing this:
> echo 1 > /sys/class/firmware/efi-capsule-file/loading
> cat capsule1 > /sys/class/firmware/efi-capsule-file/data
> cat capsule2 > /sys/class/firmware/efi-capsule-file/data
> echo 0 > /sys/class/firmware/efi-capsule-file/loading
> If so, capsule2 will be the one we will obtain in the callback function.

Here's the race:

echo 1 > /sys/class/firmware/efi-capsule-file/loading
cat capsule1 > /sys/class/firmware/efi-capsule-file/data
echo 0 > /sys/class/firmware/efi-capsule-file/loading

Kernel: Be a little slow here due to preemption or whatever.

-f /sys/class/firmware/efi-capsule-file/loading returns true
capsules_loaded == 0
Assume failure, incorrectly

Kernel: catch up and increment capsules_loaded.

If these patches get applied, then I think that the protocol needs to
be documented in Documentation/ABI.  It should say something like:

To upload an EFI capsule, do this:

Write 1 to /sys/class/firmware/efi-capsule-file/loading
Write the capsule to /sys/class/firmware/efi-capsule-file/data
Write 0 to /sys/class/firmware/efi-capsule-file/loading

Make sure that /sys/class/firmware/efi-capsule-file disappears and
comes back, perhaps by cd-ing there and waiting for all the files in
the directory to go away.

Then, and only then, read capsules_loaded to detect success.

Once you've written that doc, please seriously consider whether this
interface is justifiable.  I think it sucks.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

Powered by blists - more mailing lists