Bug#1063719: More analysis and improved patch

2024-02-13 Thread George Robbert
Hey,

Just like with the previous bug (#1026927), it looks like there's more
to this one.  Trying the patch on several more systems I run into the
same symptoms on some.  Again, these have the same symptoms but
differing causes (full patch included).  What I found was:


On an intel system, where uCode/AMD.pm was run before uCode/Intel.pm

intel_sys1# needrestart -b 
NEEDRESTART-VER: 3.6
NEEDRESTART-KCUR: 6.1.0-18-amd64
NEEDRESTART-KEXP: 6.1.0-18-amd64
NEEDRESTART-KSTA: 1
NEEDRESTART-UCSTA: 1
Use of uninitialized value $ucode_vars{"CURRENT"} in concatenation (.) or 
string at /usr/sbin/needrestart line 940.
NEEDRESTART-UCCUR: 
Use of uninitialized value $ucode_vars{"AVAIL"} in concatenation (.) or 
string at /usr/sbin/needrestart line 941.
NEEDRESTART-UCEXP: 

But we don't get the 'Use of uninitialized value' when run as
`needrestart -b -v`.  The issue here is that nr_ucode_check keeps
going after the "eval ...  ${pkg}::nr_ucode_check_real..." fails
unless $debug is set.  Thus, without $debug, thi saves off the
unintialized values from AMD.pm as "the good ones", so we get the
error.

On a VM running with an AMD processor but without package
amd64-microcode, and also directly on an AMD system with a processor new enough
that it does not have a microcode version lin amd64-microcode, I get
the following identical results.

amd_vm1# needrestart -b
NEEDRESTART-VER: 3.6
NEEDRESTART-KCUR: 6.1.0-18-amd64
NEEDRESTART-KEXP: 6.1.0-18-amd64
NEEDRESTART-KSTA: 1
NEEDRESTART-UCSTA: 1
NEEDRESTART-UCCUR: 0xa0011d1
Use of uninitialized value $ucode_vars{"AVAIL"} in concatenation (.) or 
string at /usr/sbin/needrestart line 941.
NEEDRESTART-UCEXP:


amd_sys3# needrestart -b
NEEDRESTART-VER: 3.6
NEEDRESTART-KCUR: 6.1.0-18-amd64
NEEDRESTART-KEXP: 6.1.0-18-amd64
NEEDRESTART-KSTA: 1
NEEDRESTART-UCSTA: 1
NEEDRESTART-UCCUR: 0x6006705
Use of uninitialized value $ucode_vars{"AVAIL"} in concatenation (.) or 
string at /usr/sbin/needrestart line 941.
NEEDRESTART-UCEXP: 

This comes from AMD.pm not setting $ucode_vars{"AVAIL"} if it doesn't
find any matching available versions.  compare_ucode_versions()
handles, this as expected, but leaves $ucode_vars{"AVAIL"} unset to
cause problems later when run with -b.


The attached patch fixes includes the previous patch and also fixes
these 2 issues.

Hope this helps,
George

--- /tmp/uCode.pm.dist  2024-02-13 09:20:29.236867717 -0700
+++ /usr/share/perl5/NeedRestart/uCode.pm   2024-02-13 09:33:23.099742955 
-0700
@@ -152,10 +152,15 @@
 
 # call ucode modules
 foreach my $pkg (@PKGS) {
+   eval "${pkg}::nr_ucode_init();";
+   if ( $@ ) {
+   print STDERR $@ if ($debug);
+   next;
+   }
 my @nvars;
 eval "\@nvars = ${pkg}::nr_ucode_check_real(\$debug, \$ui, 
\$processors{\$pid});";
-if ( $@ && $debug ) {
-print STDERR $@;
+if ( $@ ) {
+   print STDERR $@ if ($debug);
 $ui->progress_step;
 next;
 }
@@ -174,6 +179,10 @@
 
 $ui->progress_fin;
 
+if ( $state == NRM_CURRENT && ! grep ( ( $_ eq "AVAIL"), @vars ) ) {
+   push(@vars, "AVAIL", "unavailable");
+}
+
 return ( $state, @vars );
 }
 


Bug#1063719: needrestart -b complainst of initialized variables on some AMD and all ARM

2024-02-11 Thread George Robbert
Package: needrestart
Version: 3.6-4+deb12u1
Severity: normal
Tags: patch

Dear Maintainer,

When running 'needrestart -b' on some AMD systems I get the following
uninitialized variable warning.  It also does not report the expected
microcode version (NEEDRESTART-UCEXP).

Use of uninitialized value $ucode_vars{"AVAIL"} in concatenation (.) or string 
at /usr/sbin/needrestart line 941.

On ARM systems, it gives the following 2 uninitialized variable warnings.

Use of uninitialized value $ucode_vars{"CURRENT"} in concatenation (.) or 
string at /usr/sbin/needrestart line 940.
Use of uninitialized value $ucode_vars{"AVAIL"} in concatenation (.) or string 
at /usr/sbin/needrestart line 941.

I first thought this was a regression of bug #1026927, but find this
is actually a different bug with similar behavior.

The problem stems from NeedRestart::uCode::Intel:::nr_ucode_check_real
running "successfully" on non-intel platforms.  The reason only some
AMD systems are affected is that nr_ucode_check_real() for AMD and
Intel may be run in either order, depending on the order they are
returned by findsubmod().  If AMD::nr_ucode_check_real() is run first,
the bug does not appear.

Here is sample output from 3 systems: a "failing" AMD, a "passing"
AMD, and an arm7l (banana pi).  I've trimmed (w/ grep) the stderr from
these to only the pertinent lines, please let me know if you want the
entire output.

amd_sys1# needrestart -b -v 2>/tmp/a
NEEDRESTART-VER: 3.6
NEEDRESTART-KCUR: 6.1.0-17-amd64
NEEDRESTART-KEXP: 6.1.0-18-amd64
NEEDRESTART-KSTA: 3
NEEDRESTART-UCSTA: 1
NEEDRESTART-UCCUR: 0x600063e
NEEDRESTART-UCEXP: 
amd_sys1# grep -e NeedRestart::uCode -e uninitialized /tmp/a 
[ucode] using NeedRestart::uCode::Intel
[ucode] using NeedRestart::uCode::AMD
Use of uninitialized value $ucode_vars{"AVAIL"} in concatenation (.) or 
string at /usr/sbin/needrestart line 941.

amd_sys2# needrestart -b -v 2>/tmp/a
NEEDRESTART-VER: 3.6
NEEDRESTART-KCUR: 6.1.0-17-amd64
NEEDRESTART-KEXP: 6.1.0-18-amd64
NEEDRESTART-KSTA: 3
NEEDRESTART-UCSTA: 1
NEEDRESTART-UCCUR: 0x06000852
NEEDRESTART-UCEXP: 0x06000852
amd_sys2# grep -e NeedRestart::uCode -e uninitialized /tmp/a   
[ucode] using NeedRestart::uCode::AMD
[ucode] using NeedRestart::uCode::Intel

arm7l_sys# needrestart -b -v 2>/tmp/a
NEEDRESTART-VER: 3.6
NEEDRESTART-KCUR: 6.1.0-17-armmp-lpae
NEEDRESTART-KEXP: 6.1.0-18-armmp-lpae
NEEDRESTART-KSTA: 3
NEEDRESTART-UCSTA: 0
NEEDRESTART-SVC: irqbalance
NEEDRESTART-SVC: lircd
NEEDRESTART-SVC: lircmd
NEEDRESTART-SVC: openbsd-inetd
NEEDRESTART-SVC: ssh
arm7l_sys# grep -e uninitial -e ucode /tmp/a   
[ucode] using NeedRestart::uCode::Intel
[ucode] using NeedRestart::uCode::AMD
[ucode] #0 did not get available microcode version
Use of uninitialized value $ucode_vars{"CURRENT"} in concatenation (.) or 
string at /usr/sbin/needrestart line 940.
Use of uninitialized value $ucode_vars{"AVAIL"} in concatenation (.) or 
string at /usr/sbin/needrestart line 941.


The problem comes from uCode::Intel::nr_ucode_check_real() still not
dying on non-Intel processors, and actually returning a "CURRENT"
microcode version on AMD processors.  This looks to be checked for in
uCode::Intel::nr_ucode_init(), but this function is never called.

The following patch adds a call to this so that nr_ucode_check_real()
is only called for architectures that successfully pass
nr_ucode_init().  An alternative would be to modify
uCode::Intel::nr_ucode_check_real() to do the equivalent checks like
uCode::AMD does.



--- /tmp/uCode.pm.dist  2024-02-11 09:28:27.051119002 -0700
+++ uCode.pm2024-02-11 10:14:26.905533440 -0700
@@ -152,6 +152,11 @@
 
 # call ucode modules
 foreach my $pkg (@PKGS) {
+   eval "${pkg}::nr_ucode_init();";
+   if ( $@ ) {
+   print STDERR $@ if ($debug);
+   next;
+   }
 my @nvars;
 eval "\@nvars = ${pkg}::nr_ucode_check_real(\$debug, \$ui, 
\$processors{\$pid});";
 if ( $@ && $debug ) {






-- Package-specific info:
needrestart output:
Your outdated processes:
emacs[9574, 6843, 10275, 3922, 6197]



-- System Information:
Debian Release: 12.0
Architecture: i386 (i686)

Kernel: Linux 6.1.0-17-686-pae (SMP w/6 CPU threads; PREEMPT)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=ANSI_X3.4-1968) 
(ignored: LC_ALL set to C), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: sysvinit (via /sbin/init)

Versions of packages needrestart depends on:
ii  binutils   2.40-2
ii  dpkg   1.21.22
ii  gettext-base   0.21-12
ii  libintl-perl   1.33-1
ii  libmodule-find-perl0.16-2
ii  libmodule-scandeps-perl1.31-2
ii  libproc-processtable-perl  0.634-1+b2
ii  libsort-naturally-perl 1.03-4
ii  libterm-readkey-perl

Bug#1026927: More analysis and improved patch

2023-01-14 Thread George Robbert
Hey,

I think I found why you still get the error after applying the patch.
The same thing happened to me on another of my systems.  The
difference was whether the package amd64-microcode was installed or
not.  I think I have a patch (attached) for both.  Here's what I found.



It looks like there are actually 2 related bugs here.  The first one
(which my original patch for line 182 addresses) applies when there is
microcode available.  The second, when there is not.  Here's what I've
found:

Both of these are exposed on line 904 of /usr/sbin/needrestart

print "NEEDRESTART-UCEXP: $ucode_vars{AVAIL}\n";

when $ucode_vars{AVAIL} never gets set.  There are 2 ways this can
happen.  The second (which I just found), comes when AMD.pm finds an
AMD cpu, but does not find microcode for it (namely _scan_ucodes does
not find a matching file in /lib/firmware/amd-ucode/microcode_*.bin).
This will, under -v, print

"$LOGPREF #$info->{processor} no ucode updates available\n"

and return NRM_CURRENT (indicating no reboot for microcode needed),
but still leave $ucode_vars{AVAIL} unset.  The fix is to make sure
that it is set even when there are no microcode updates available (on
line 176).


The first (original patch) bug comes when needrestart finds a matching
microcode.  There, on line 182 of /usr/share/perl5/NeedRestart/uCode/AMD.pm
the line ends with a comma (,) instead of a semicolon (;), which means
that the assignment to $ucode_vars{AVAIL} is subsumed under the if ($debug)
condoling the printing of microcode version under.  Thus,
$ucode_vars{AVAIL} is only set under needrestart -v.


The attached patch fixes both of these cases.

Thanks,
George

--- /tmp/AMD.pm 2023-01-14 09:09:28.324414456 -0700
+++ /usr/share/perl5/NeedRestart/uCode/AMD.pm   2023-01-14 09:11:39.210705528 
-0700
@@ -173,13 +173,13 @@
 _scan_ucodes();
 }
 
-my %vars = ( CURRENT => sprintf( "0x%08x", $ucode ), );
+my %vars = ( CURRENT => sprintf( "0x%08x", $ucode ), AVAIL => 
"unavailable");
 
 # check for microcode updates
 if ( exists( $_ucodes->{cpuid}->{$cpuid} ) ) {
 my $prid = $_ucodes->{cpuid}->{$cpuid};
 if ( exists( $_ucodes->{prid}->{$prid} ) ) {
-$vars{AVAIL} = sprintf( "0x%08x", $_ucodes->{prid}->{$prid} ),
+$vars{AVAIL} = sprintf( "0x%08x", $_ucodes->{prid}->{$prid} );
 
   print STDERR "$LOGPREF #$info->{processor} found ucode 
$vars{AVAIL}\n" if ($debug);
 if ( $_ucodes->{prid}->{$prid} > $ucode ) {


Bug#1026927: needrestart -b on AMD processors complains of perl uninitialized variable

2023-01-09 Thread George Robbert
On Fri, 06 Jan 2023 01:56:33 -0700, Patrick Matthäi wrote:
> Thanks for your investigation. I have tested it on my (sid/unstable) AMD
> notebook and I can reproduce your issue with needrestart from stable.
> But your patch does not produce another result for me. Are you sure,
> that you didnt changed something else?

Color me confused.  That file (in fact that one character) is all I
changed.  I tried it again and the patch seems to work for me.  What I
did was:

apt-get install --reinstall needrestart
needrestart -b
# error message
apply patch
needrestart -b  
# no error here

I'm attaching a full `script` of that attempt in case there's
something significant in there that the above summary leaves out.
That also has md5sum of AMD.pm before and after the patch in case that
helps point to "what's different" between our two runs.

If there's any more info I can supply, please let me know.  Thanks for
looking into this.

Thanks,
George
Script started on 2023-01-09 12:32:39-07:00 [TERM="xterm" TTY="/dev/pts/0" 
COLUMNS="80" LINES="24"]
root@xecty# PS1='# '
# apt-get install --reinstall needrestart

Reading package lists... 0%

Reading package lists... 100%

Reading package lists... Done


Building dependency tree... 0%

Building dependency tree... 0%

Building dependency tree... 50%

Building dependency tree... 50%

Building dependency tree... Done


Reading state information... 0% 

Reading state information... 0%

Reading state information... Done

0 upgraded, 0 newly installed, 1 reinstalled, 0 to remove and 0 not upgraded.
Need to get 0 B/62.4 kB of archives.
After this operation, 0 B of additional disk space will be used.
(Reading database ... 
(Reading database ... 5%
(Reading database ... 10%
(Reading database ... 15%
(Reading database ... 20%
(Reading database ... 25%
(Reading database ... 30%
(Reading database ... 35%
(Reading database ... 40%
(Reading database ... 45%
(Reading database ... 50%
(Reading database ... 55%
(Reading database ... 60%
(Reading database ... 65%
(Reading database ... 70%
(Reading database ... 75%
(Reading database ... 80%
(Reading database ... 85%
(Reading database ... 90%
(Reading database ... 95%
(Reading database ... 100%
(Reading database ... 59046 files and directories currently installed.)
Preparing to unpack .../needrestart_3.5-4+deb11u2_all.deb ...
Unpacking needrestart (3.5-4+deb11u2) over (3.5-4+deb11u2) ...
Setting up needrestart (3.5-4+deb11u2) ...
Processing triggers for man-db (2.9.4-2) ...
NEEDRESTART-VER: 3.5
NEEDRESTART-KCUR: 5.10.0-20-amd64
NEEDRESTART-KEXP: 5.10.0-20-amd64
NEEDRESTART-KSTA: 1
NEEDRESTART-UCSTA: 1
NEEDRESTART-UCCUR: 0x0327
Use of uninitialized value $ucode_vars{"AVAIL"} in concatenation (.) or string 
at /usr/sbin/needrestart line 904.
NEEDRESTART-UCEXP: 
# 
# 
# needrestart -b
NEEDRESTART-VER: 3.5
NEEDRESTART-KCUR: 5.10.0-20-amd64
NEEDRESTART-KEXP: 5.10.0-20-amd64
NEEDRESTART-KSTA: 1
NEEDRESTART-UCSTA: 1
NEEDRESTART-UCCUR: 0x0327
Use of uninitialized value $ucode_vars{"AVAIL"} in concatenation (.) or string 
at /usr/sbin/needrestart line 904.
NEEDRESTART-UCEXP: 
#  cd /usr/share/perl5/NeedRestart/uCode/
# cat ~ghr/needrestart.patch 
--- /tmp/AMD.pm 2022-12-22 11:00:14.589106185 -0700
+++ /usr/share/perl5/NeedRestart/uCode/AMD.pm   2022-12-22 11:00:24.329046436 
-0700
@@ -179,7 +179,7 @@
 if ( exists( $_ucodes->{cpuid}->{$cpuid} ) ) {
 my $prid = $_ucodes->{cpuid}->{$cpuid};
 if ( exists( $_ucodes->{prid}->{$prid} ) ) {
-$vars{AVAIL} = sprintf( "0x%08x", $_ucodes->{prid}->{$prid} ),
+$vars{AVAIL} = sprintf( "0x%08x", $_ucodes->{prid}->{$prid} );
 
   print STDERR "$LOGPREF #$info->{processor} found ucode 
$vars{AVAIL}\n" if ($debug);
 if ( $_ucodes->{prid}->{$prid} > $ucode ) {
# md5sum AMD.pm
aa911d7cb8c97f464db89f0252ecaf73  AMD.pm
# patch -p6 <~ghr/needrestart.patch 
patching file AMD.pm
#  md5sum AMD.pm
2be0d48086c482f6425e9626f73afbb9  AMD.pm
# needrestart -b
NEEDRESTART-VER: 3.5
NEEDRESTART-KCUR: 5.10.0-20-amd64
NEEDRESTART-KEXP: 5.10.0-20-amd64
NEEDRESTART-KSTA: 1
NEEDRESTART-UCSTA: 1
NEEDRESTART-UCCUR: 0x0327
NEEDRESTART-UCEXP: 0x0327
# exit

Script done on 2023-01-09 12:33:54-07:00 [COMMAND_EXIT_CODE="0"]


Bug#1026927: needrestart -b on AMD processors complains of perl uninitialized variable

2022-12-23 Thread George Robbert
Package: needrestart
Version: 3.5-4+deb11u2
Severity: normal

Dear Maintainer,

When running 'needrestart -b' on an AMD system, I get the following
uninitialized variable warning.  It also does not report the expected
microcode version (NEEDRESTART-UCEXP).  See output: section below for
example output of needrestart -b.

Use of uninitialized value $ucode_vars{"AVAIL"} in concatenation (.) or string 
at /usr/sbin/needrestart line 904.

This perl warning disappears when adding the -v option (needrestart -b -v),
and the correct value is reported for NEEDRESTART-UCEXP.

The processor, in this system, is:

vendor_id   : AuthenticAMD
cpu family  : 18
model   : 1
model name  : AMD A4-3400 APU with Radeon(tm) HD Graphics
stepping: 0
microcode   : 0x327



It looks to me like the problem is that line 182 of
/usr/share/perl5/NeedRestart/uCode/AMD.pm ends in a comma (,) instead
of a semicolon (;).  This means that assignment is subsumed into the
next line which is under if ($debug).

See the attached patch.

It also looks to me like this is similar symptoms with a different
root cause from bug #973050.


Thanks,
George Robbert


-- Package-specific info:
needrestart output:



-- System Information:
Debian Release: 11.1
Architecture: amd64 (x86_64)

Kernel: Linux 5.10.0-20-amd64 (SMP w/2 CPU threads)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=ANSI_X3.4-1968) 
(ignored: LC_ALL set to C), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: sysvinit (via /sbin/init)

Versions of packages needrestart depends on:
ii  binutils   2.35.2-2
ii  dpkg   1.20.12
ii  gettext-base   0.21-4
ii  libintl-perl   1.26-3+deb11u1
ii  libmodule-find-perl0.15-1
ii  libmodule-scandeps-perl1.30-1
ii  libproc-processtable-perl  0.59-2+b1
ii  libsort-naturally-perl 1.03-2
ii  libterm-readkey-perl   2.38-1+b2
ii  perl   5.32.1-4+deb11u2
ii  xz-utils   5.2.5-2.1~deb11u1

Versions of packages needrestart recommends:
ii  libpam-elogind [libpam-systemd]  246.10-2
ii  sysvinit-core2.96-7+devuan2

Versions of packages needrestart suggests:
pn  iucode-tool  
pn  needrestart-session | libnotify-bin  

-- Configuration Files:
/etc/apt/apt.conf.d/99needrestart changed:
DPkg::Post-Invoke {"test -x /usr/lib/needrestart/apt-pinvoke && 
/usr/lib/needrestart/apt-pinvoke -b || true"; };

/etc/needrestart/hook.d/20-rpm [Errno 2] No such file or directory: 
'/etc/needrestart/hook.d/20-rpm'

-- no debconf information
--- /tmp/AMD.pm 2022-12-22 11:00:14.589106185 -0700
+++ /usr/share/perl5/NeedRestart/uCode/AMD.pm   2022-12-22 11:00:24.329046436 
-0700
@@ -179,7 +179,7 @@
 if ( exists( $_ucodes->{cpuid}->{$cpuid} ) ) {
 my $prid = $_ucodes->{cpuid}->{$cpuid};
 if ( exists( $_ucodes->{prid}->{$prid} ) ) {
-$vars{AVAIL} = sprintf( "0x%08x", $_ucodes->{prid}->{$prid} ),
+$vars{AVAIL} = sprintf( "0x%08x", $_ucodes->{prid}->{$prid} );
 
   print STDERR "$LOGPREF #$info->{processor} found ucode 
$vars{AVAIL}\n" if ($debug);
 if ( $_ucodes->{prid}->{$prid} > $ucode ) {


Bug#958353: sleepd: fails to correctly read acpi subsystem version

2020-04-20 Thread George Robbert
Package: sleepd
Version: 2.10
Severity: normal
Tags: patch

Dear Maintainer,

When running with a newer kernel (4.19.0-8-amd64 from buster, though
this also occurs with 5.4.0-0.bpo.4-amd64 from backports), and without
upower installed, sleepd fails to start, complaining

ACPI subsystem 2018081 too is old, consider upgrading to 20011018.

It appears that with these kernels, /sys/module/acpi/parameters/acpica_version
no longer ends with a newline character.  This file actually contains
"20180810".  Note that the final '0' character is discarded.

It seems that acpi.c:get_acpi_file() is erroneously truncating the
last byte when reading acpi files.  This was never exposed earlier, as
the truncated byte was a newline, not part of the version number.

The following patch seems to fix the problem.


--- acpi.c.dist 2018-09-16 03:13:08.0 -0600
+++ acpi.c  2020-04-18 08:13:44.946652962 -0600
@@ -67,7 +67,8 @@
fd = open(file, O_RDONLY);
if (fd == -1) return NULL;
end = read(fd, buf, sizeof(buf));
-   buf[end-1] = '\0';
+   buf[end] = '\0';
close(fd);
return buf;
 }




-- System Information:
Debian Release: 10.3
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 5.4.0-0.bpo.4-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=ANSI_X3.4-1968) 
(ignored: LC_ALL set to C), LANGUAGE=en_US.UTF-8 (charmap=ANSI_X3.4-1968) 
(ignored: LC_ALL set to C)
Shell: /bin/sh linked to /bin/dash
Init: sysvinit (via /sbin/init)

Versions of packages sleepd depends on:
ii  libc62.28-10
ii  libglib2.0-0 2.58.3-2+deb10u2
ii  libupower-glib3  0.99.10-1
ii  lsb-base 10.2019051400

Versions of packages sleepd recommends:
ii  pm-utils  1.4.1-18
pn  upower

sleepd suggests no packages.

-- Configuration Files:
/etc/default/sleepd changed:
PARAMS="-b 2 -s '/etc/acpi/sleep_suspend.sh sleep'"


-- no debconf information