Bug#680263: Illegal number - maybe already fixed

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Bug#680263: Illegal number - maybe already fixed

Marcos Fouces-2
Hello

Since 0.50 release, the test to check the name of the file (ksysm or
kallsyms) is the following:

 KALLSYMS="/proc/kallsyms"
 [ -f /proc/ksysm ] && KALLSYMS="/proc/$KALLSYMS"

so i believe that it is not a problem anymore. Anyway I cannot test it
with an kernel older enough to use /proc/ksysm.

If you could test it, please tell me if it is really fixed.

Greetings,

Marcos

Reply | Threaded
Open this post in threaded view
|

Bug#680263: Illegal number - maybe already fixed

Vincent Lefevre-10
On 2019-05-03 16:17:06 +0200, Marcos Fouces wrote:
> Since 0.50 release, the test to check the name of the file (ksysm or
> kallsyms) is the following:
>
>  KALLSYMS="/proc/kallsyms"
>  [ -f /proc/ksysm ] && KALLSYMS="/proc/$KALLSYMS"
>
> so i believe that it is not a problem anymore. Anyway I cannot test it
> with an kernel older enough to use /proc/ksysm.

The error was related to the "uname -r", and the script still has an
occurrence, which needs to be checked. But the code in 0.50-4+deb9u1
(stretch) and 0.52-3 (unstable/testing) is:

VERSION=`${uname} -r`
if [ "${SYSTEM}" != "FreeBSD" -a ${SYSTEM} != "OpenBSD" ] ; then
   V=4.4
else
   V=`echo $VERSION| ${sed} -e 's/[-_@].*//'| ${awk} -F . '{ print $1 "." $2 $3 }'`
fi

In the past, one had some kernel numbers of the form 3.10-2-amd64
(i.e. with 2 numbers instead of 3 before the first hyphen), but
the code above handles that ($3 would be empty, which is OK).
So V has always the form of a floating-point number: digits,
followed by a period, followed by digits. This is what the lkm
function expects, the only place where $V is used.

However, the code still appears to be incorrect, in this lkm function:

    if [  \( "${SYSTEM}" = "Linux"  -o \( "${SYSTEM}" = "FreeBSD" -a \
       `echo ${V} | ${awk} '{ if ($1 > 4.3 || $1 < 6.0) print 1; else print 0 }'` -eq 1 \) \) -a "${ROOTDIR}" = "/" ]; then

If the system is FreeBSD, there is a test on the value of V, which
is $1 in the awk script:

  $1 > 4.3 || $1 < 6.0

There are 2 bugs here. :) First, I assume that || should have been &&,
otherwise the test is always true! Then I assume that the goal of this
test is to check if the kernel version is larger than 4.3 (or 4.3.0)
and smaller than 6.0 (or 6.0.0). But this test will fail to give the
correct answer for versions from 4.10 to 4.30. According to

  https://en.wikipedia.org/wiki/FreeBSD

4.10 and 4.11 existed, but only from 2004 to 2007.

Nowadays, the test could be simplified by someone who knows FreeBSD.
Currently it is already true, while it seems to be expected to be
always false.

Since kFreeBSD is no longer official in Debian, the FreeBSD issue is
actually a different bug, and no kFreeBSD users have ever complained,
I suggest to close this bug.

--
Vincent Lefèvre <[hidden email]> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Reply | Threaded
Open this post in threaded view
|

Bug#680263: Illegal number - maybe already fixed

Marcos Fouces-2
Hello Vincent,

i believe that we should don't care about bugs that affect kernels
before oldstable release.

In this case, the script test the existence of /proc/ksyms and set the
variable KALLSYMS to /proc/kallsyms only if it doesn't exist so the
kernel version should not be relevant. AFAIK, the name of the file was
changed at some point during 2.6 releases of Linux kernel.

FTR. I share your opinion about closing this bug.

Greetings,

Marcos