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

[Marius Gedminas]

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)]

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.

[Leann Ogasawara (leannogasawara)]

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.

[Dimitri John Ledkov (xnox)]

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

[Henrique de Moraes Holschuh (hmh)]

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

[Dimitri John Ledkov (xnox)]

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.

[Henrique de Moraes Holschuh (hmh)]

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

[Dimitri John Ledkov (xnox)]

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...

[Dimitri John Ledkov (xnox)]

I don't think this is high anymore.
Kernel now depends and pulls in microcode packages, and kernel does trigger reboot require notification.
Imho, it is sufficient enough to require reboot from the kernel alone. Since there are frequent enough kernel updates.

[Steve Beattie (sbeattie)]

The needrestart package has some sophisticated logic to detect whether the system needs to be booted to get an updated microcode applied (needrestart -w is how it can be invoked directly to report on microcode status). The needrestart package is a bit much to be included as a dependency or even a recommends for the intel-microcode package, but re-using some of the logic might be appropriate.