Microcode updates require a reboot to apply, but package postinst doesn't touch /run/reboot-required

Bug #1742903 reported by Marius Gedminas on 2018-01-12
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
intel-microcode (Ubuntu)
High
Dimitri John Ledkov 🌈
Bionic
High
Dimitri John Ledkov 🌈

Bug Description

The intel-microcode postinst script prints this text to standard error

    intel-microcode: microcode will be updated at next boot

but tools like unattended-upgrades don't parse stderr, they expect a /run/reboot-required to exist to tell them that a reboot is needed to complete the update.

Can you fix the postinst script to touch /run/reboot-required?

Dean Henrichsmeyer (dean) wrote :

This will be taken care of because the microcode will be pulled in as a dependency on the kernel containing mitigation patches and that will require the reboot.

For awareness, the kernel team is preparing updates including a package dependency on the intel-microcode. We have not pushed that out yet due to reported regressions with the intel-microcode updates:

https://newsroom.intel.com/news/intel-security-issue-update-addressing-reboot-issues/

We have reached out to Intel for information/guidance before we proceed.

Changed in intel-microcode (Ubuntu):
status: New → Triaged
importance: Undecided → High
tags: added: packaging
tags: added: rls-bb-incoming
Steve Langasek (vorlon) on 2018-03-22
tags: removed: rls-bb-incoming

but it may need to be more smart; e.g. check the new microcode revisions vs currently loaded; to see if there was an update to microcode for this system, actually

Changed in intel-microcode (Ubuntu Bionic):
assignee: nobody → Dimitri John Ledkov (xnox)
milestone: none → ubuntu-18.03

On Thu, 22 Mar 2018, Dimitri John Ledkov wrote:
> but it may need to be more smart; e.g. check the new microcode revisions
> vs currently loaded; to see if there was an update to microcode for this
> system, actually

That would be why I did not do it in the Debian package (yet). It is
hard to make it fool-proof, and it will be really annoying [to the user]
if the package triggers unnecessary reboots (which will be the *rule* on
older processors that rarely get updates).

If you have good ideas on how to do it, I am listening...

--
  Henrique Holschuh

tags: added: id-5ab3cd314b544033e99b8f0b

Something like:

[[ `iucode_tool -s $(sudo iucode_tool --scan-system=2 2>&1 | sed 's/.*\(0x.*\)$/\1/') -l /lib/firmware/intel-ucode/ | sed -n 's/.* rev \(0x....\),.*/\1/p'` -eq `sudo cat /sys/devices/system/cpu/cpu0/microcode/version` ]] || echo 'Trigger reboot'

should be good enough.... with bashism... as the kernel file is 0x1f, yet the tool reports 0x001f.

There will be false negatives, and false positives, but overall should be better than the current behavior.

Obviously, no need for sudo, if inside the postinst.

On Mon, 26 Mar 2018, Dimitri John Ledkov wrote:
> Something like:
>
> [[ `iucode_tool -s $(sudo iucode_tool --scan-system=2 2>&1 | sed
> 's/.*\(0x.*\)$/\1/') -l /lib/firmware/intel-ucode/ | sed -n 's/.* rev
> \(0x....\),.*/\1/p'` -eq `sudo cat
> /sys/devices/system/cpu/cpu0/microcode/version` ]] || echo 'Trigger
> reboot'
>
> should be good enough.... with bashism... as the kernel file is 0x1f,
> yet the tool reports 0x001f.

Bashisms aren't a problem, as long as the script declares to be a bash
script. Those numbers have to be properly parsed, so using bash is a good
solution. They are guaranteed to be 32-bit, too, which doesn't get in
the way (bash math is 32-bit).

The use of --scan-system=2 is a problem (slow when there are many CPUs,
requires a kernel module to be loaded), and I don't understand why the
whole dance to feed its output to iucode-tool -s. Looks like just
"iucode_tool -S" would do what is required *and* handle systems in
mixed stepping configurations better...

> There will be false negatives, and false positives, but overall should
> be better than the current behavior.

There are a lot of corner cases. Also, I am worried that it would have
*both* false positives and false negatives. False positives are
annoying and cause extra downtime (needlessly request a reboot), but
false negatives are *bad* once people start trusting the package to
request a reboot when one would be required.

For example, one has to consider whether it should be checking the
initrds as well or not. Note that while fortunately, the iucode_tool
-tr loader option can read the initrd directly, the intel-microcode and
iucode-tool packages have no idea where the initrd is, since this
knowledge belongs to other layers (initramfs-tools/dracut, bootloader).

Anyway, the script must learn to deal with the cases where the microcode
update disappeared as well, or it will break badly.

> Obviously, no need for sudo, if inside the postinst.

Indeed...

--
  Henrique Holschuh

Download full text (3.6 KiB)

On 26 March 2018 at 13:07, Henrique de Moraes Holschuh
<email address hidden> wrote:
> On Mon, 26 Mar 2018, Dimitri John Ledkov wrote:
>> Something like:
>>
>> [[ `iucode_tool -s $(sudo iucode_tool --scan-system=2 2>&1 | sed
>> 's/.*\(0x.*\)$/\1/') -l /lib/firmware/intel-ucode/ | sed -n 's/.* rev
>> \(0x....\),.*/\1/p'` -eq `sudo cat
>> /sys/devices/system/cpu/cpu0/microcode/version` ]] || echo 'Trigger
>> reboot'
>>
>> should be good enough.... with bashism... as the kernel file is 0x1f,
>> yet the tool reports 0x001f.
>
> Bashisms aren't a problem, as long as the script declares to be a bash
> script. Those numbers have to be properly parsed, so using bash is a good
> solution. They are guaranteed to be 32-bit, too, which doesn't get in
> the way (bash math is 32-bit).
>
> The use of --scan-system=2 is a problem (slow when there are many CPUs,
> requires a kernel module to be loaded), and I don't understand why the
> whole dance to feed its output to iucode-tool -s. Looks like just
> "iucode_tool -S" would do what is required *and* handle systems in
> mixed stepping configurations better...
>

$(--scan-system) -> find the current processor signature e.g. 0x000306a9

Pass that as the selector, to the listing of all microcodes:

iucode_tool -s 0x000306a9 -l /lib/firmware/intel-ucode

Thus find me the available revision of ucode on disk.

Compare that with currently loaded revision of ucode, as reported via
kernel sysfs:
cat /sys/devices/system/cpu/cpu0/microcode/version

if the two do not match (on-disk vs loaded onto the cpu) -> request
reboot. This will bring up cases, of any missmatch, eg.
older/newer/dowgrade/upgrade.

Thus I am comparing runtime - what the cpu is reporting as loaded (via
sysfs), versus what I could have loaded from disk, if I reboot with a
fresh initramfs generated from the ucode on disk.

Note this will run only at postinst of intel-microcode package. Thus
if there is a false positive, this will only be triggered once, not at
like every call to `update-initramfs -u`. (for the cases of multiple
cpus / different cpu ids / etc).

I guess we can run above for every cpuX in the
/sys/devices/system/cpu/ directory, to catch ucode updates to all
cores.

>> There will be false negatives, and false positives, but overall should
>> be better than the current behavior.
>
> There are a lot of corner cases. Also, I am worried that it would have
> *both* false positives and false negatives. False positives are
> annoying and cause extra downtime (needlessly request a reboot), but
> false negatives are *bad* once people start trusting the package to
> request a reboot when one would be required.
>

I am not worried about that. One is expected to reboot approximately
every three week on Ubuntu anyway, due to full kernel updates.

> For example, one has to consider whether it should be checking the
> initrds as well or not. Note that while fortunately, the iucode_tool
> -tr loader option can read the initrd directly, the intel-microcode and
> iucode-tool packages have no idea where the initrd is, since this
> knowledge belongs to other layers (initramfs-tools/dracut, bootloader).
>

I am checking currently loaded, versus what...

Read more...

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers