[PATCH] util-linux/sys-utils dmesg -r LOG_MAKEPRI needs fac << 3

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Edward Chron <echron@xxxxxxxxx>

Submission to Project: util-linux
Open Incident: #2325 at github.com/util-linux/util-linux/issues/2325
Component: util-linux/sys-utils
File: dmesg.c
Code level patch applied against: 2.39 - latest code pulled from
           git.github.com:util-linux/util-linux.git

BUG: The facility field passed to macro from /usr/include/sys/syslog.h
     LOG_MAKEPRI(fac, pri) by dmesg -r needs to have fac argument
     shifted 3 bit to the left to return a valid raw valid. The lower
     3 bits for a raw value are used by the loglevel priority (pri)
     field, so the facility bits can only occupy the bits above the
     bits used to hold the pri field value.

The dmesg -r command produces the incorrect output for the raw value
for the or'ed combination of the facility | loglevel priority that gets
produced by the LOG_MAKEPRI macro. That macro is defined as:

    #define LOG_MAKEPRI(fac, pri) ((fac) | (pri))

which is defined in the current glibc code in /usr/include/sys/syslog.h
and is used only in the dmesg -r (raw output option) command to form the
raw value for facility or'ed with loglevel priority and displayed as:

    <#>[#######.######] ...

where the # in <#> contains the output from the LOG_MAKEPRI macro.

The lower 3 bits are reserved for the loglevel priority 0-7
and the bits above that are for the facility value, so the facility
index should be shifted to the left three bits and or'ed with the
loglevel priority.

In the glibc file: /usr/include/sys/syslog.h the macro LOG_MAKEPRI is
defined as:

        #define LOG_MAKEPRI(fac, pri) ((fac) | (pri)

and returns the wrong facility and loglevel priority values, ideally it
should be defined as:

        #define LOG_MAKEPRI(fac, pri) ((fac << 3) | (pri))

to return the correct raw value.

We checked with glibc developement and the LOG_MAKEPRI macro is correct
as is and can't be changed as it used by *BSD as is so the solution for
dmesg -r is to shift the facility index left by 3 bits as input to the
LOG_MAKEPRI macro. That is what glibc development recommended.
(For reference, see glibc bugzilla Bug 30563)

We can front end the LOG_MAKEPRI macro with a macro that shifts the
facility by the needed 3 bits which we've added to dmesg.c:

    #define LOG_RAW_FAC_PRI(fac, pri) LOG_MAKEPRI((fac << 3), (pri))

This has been tested and works correctly to produce the correct raw
mode value for Facility or'ed together with Loglevel priority.

You can verify that this fix works correctly.

We can test by adding several records to /dev/kmsg like this:

    echo "<14> Test Message Facility 8 Loglevel 6" >> /dev/kmsg
    echo "<15> Test Message Facility 8 Loglevel 7" >> /dev/kmsg
    echo "<30> Test Message Facility 24 Loglevel 6" >> /dev/kmsg
    echo "<31> Test Message Facility 24 Loglevel 7" >> /dev/kmsg

these commands add 4 records to the dmesg buffer. Then when we print the
records by cat'ing /dev/kmsg or using the dmesg command several ways:

    -bash-4.2# cat /dev/kmsg | grep "Test Message Facility"
    14,1114,495317239,-; Test Message Facility 8 Loglevel 6
    15,1115,503340779,-; Test Message Facility 8 Loglevel 7
    30,1116,643374764,-; Test Message Facility 24 Loglevel 6
    31,1117,657165117,-; Test Message Facility 24 Loglevel 7

    -bash-4.2# dmesg -x | grep "Test Message Facility"
    user  :info  : [  495.317239]  Test Message Facility 8 Loglevel 6
    user  :debug : [  503.340779]  Test Message Facility 8 Loglevel 7
    daemon:info  : [  643.374764]  Test Message Facility 24 Loglevel 6
    daemon:debug : [  657.165117]  Test Message Facility 24 Loglevel 7

    -bash-4.2# dmesg -S -x | grep "Test Message Facility"
    user  :info  : [  495.317239]  Test Message Facility 8 Loglevel 6
    user  :debug : [  503.340779]  Test Message Facility 8 Loglevel 7
    daemon:info  : [  643.374764]  Test Message Facility 24 Loglevel 6
    daemon:debug : [  657.165117]  Test Message Facility 24 Loglevel 7

   -bash-4.2# dmesg -S -r | grep "Test Message Facility"
   <14>[  495.317239]  Test Message Facility 8 Loglevel 6
   <15>[  503.340779]  Test Message Facility 8 Loglevel 7
   <30>[  643.374764]  Test Message Facility 24 Loglevel 6
   <31>[  657.165117]  Test Message Facility 24 Loglevel 7

All the above methods agree in their output as expected.
However, running dmesg -r does not agree.

dmesg -r erronously produces:
----------------------------

    -bash-4.2# dmesg -r | grep "Test Message Facility"
    <7>[  495.317239]  Test Message Facility 8 Loglevel 6
    <7>[  503.340779]  Test Message Facility 8 Loglevel 7
    <7>[  643.374764]  Test Message Facility 24 Loglevel 6
    <7>[  657.165117]  Test Message Facility 24 Loglevel 7


However, if we run the dmesg -r command using the new front end macro
LOG_RAW_FAC_PRI(fac, pri) we do get the correct output:

Here is the corrected dmesg -r output:
-------------------------------------

    -bash-4.2# dmesg -r | grep "Test Message Facility"
    <14>[  495.317239]  Test Message Facility 8 Loglevel 6
    <15>[  503.340779]  Test Message Facility 8 Loglevel 7
    <30>[  643.374764]  Test Message Facility 24 Loglevel 6
    <31>[  657.165117]  Test Message Facility 24 Loglevel 7

shifting the facility index value by 3 bits in the LOG_RAW_FAC_PRI macro
provides the correct ouput as shown. All the other commands produce the
same output so now they are all in agreement.

Signed-off-by: Ivan Delalande <colona@xxxxxxxxxx>
Signed-off-by: Edward Chron <echron@xxxxxxxxxx>
---
 sys-utils/dmesg.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sys-utils/dmesg.c b/sys-utils/dmesg.c
index 717cc588b..02358e449 100644
--- a/sys-utils/dmesg.c
+++ b/sys-utils/dmesg.c
@@ -125,6 +125,7 @@ static const struct dmesg_name level_names[] =
  * shifted code :-)
  */
 #define FAC_BASE(f)	((f) >> 3)
+#define LOG_RAW_FAC_PRI(fac, pri)	LOG_MAKEPRI((fac << 3), (pri))
 
 static const struct dmesg_name facility_names[] =
 {
@@ -968,7 +969,7 @@ static void print_record(struct dmesg_control *ctl,
 	if (ctl->raw) {
 		ctl->indent = snprintf(tsbuf, sizeof(tsbuf),
 				       "<%d>[%5ld.%06ld] ",
-				       LOG_MAKEPRI(rec->facility, rec->level),
+				       LOG_RAW_FAC_PRI(rec->facility, rec->level),
 				       (long) rec->tv.tv_sec,
 				       (long) rec->tv.tv_usec);
 		goto full_output;
@@ -1052,7 +1053,7 @@ full_output:
 			ul_jsonwrt_value_s(&ctl->jfmt, "fac", facility_names[rec->facility].name);
 			ul_jsonwrt_value_s(&ctl->jfmt, "pri", level_names[rec->level].name);
 		} else
-			ul_jsonwrt_value_u64(&ctl->jfmt, "pri", LOG_MAKEPRI(rec->facility, rec->level));
+			ul_jsonwrt_value_u64(&ctl->jfmt, "pri", LOG_RAW_FAC_PRI(rec->facility, rec->level));
 	}
 
 	/* Output the timestamp buffer */
-- 
2.41.0




[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux