Atoptool / atop

System and process monitor for Linux
GNU General Public License v2.0
792 stars 109 forks source link

cgroup field should have backslashes escaped in JSON output #300

Closed N-Storm closed 4 months ago

N-Storm commented 4 months ago

In json.c v2.10.0 adds cgroup field to JSON output of PR* labels. Which prints cgrpath variable. On most modern Linux systems with SystemD slices could include backslash escaped codes. For example Debian 12 based system:

# realpath system-systemd\\x2dcryptsetup.slice
/usr/lib/systemd/system/system-systemd\x2dcryptsetup.slice

These, when printed in JSON output as is produce incorrect JSON:

$ ./atop -V
Version: 2.10.0 - 2024/04/09 12:44:41     <gerlof.langeveld@atoptool.nl>

# Break output into strings on ',' to make finding point of error easier:
$ ./atop -J PRC 1 1 | sed -E 's/,/,\n/g' | jq
parse error: Invalid escape at line 1383, column 82
shirov.p@pshirov-projects1:~/SYSADMIN-16852/atop-work$ ./atop -J PRC 1 1 | sed -E 's/,/,\n/g' | sed -n 1383p
 "cgroup": "/system.slice/system-serial\x2dgetty.slice/serial-getty@ttyS0.service"}

These should be escaped as '\' to produce valid JSON:

$ ./atop -J PRC 1 1 | sed -E 's/,/,\n/g' | sed -E 's/\\x2d/\\\\x2d/g' | jq | grep 'x2d'
      "cgroup": "/system.slice/system-serial\\x2dgetty.slice/serial-getty@ttyS0.service"
      "cgroup": "/system.slice/system-serial\\x2dgetty.slice/serial-getty@ttyS1.service"

So char *cgrpath should double backslashes in json.c before printing.

N-Storm commented 4 months ago

One of the possible fixes example:

--- a/json.c
+++ b/json.c
@@ -1256,8 +1256,8 @@ json_print_PRC(char *hp, struct sstat *ss,
                          struct tstat *ps, int nact,
                         struct cgchainer *cs, int ncgroups)
 {
-       register int    i;
-       char            *cgrpath;
+       register int    i, backslashes = 0;
+       char            *cgrpath, *cgrout;

         printf(", %s: [", hp);

@@ -1268,10 +1268,27 @@ json_print_PRC(char *hp, struct sstat *ss,
                        printf(", ");
                }

-               if (supportflags & CGROUPV2 && ps->gen.cgroupix != -1)
+               if (supportflags & CGROUPV2 && ps->gen.cgroupix != -1) {
                        cgrpath = cggetpath((cs + ps->gen.cgroupix), cs);
+                       for (const char* p = cgrpath; *p != '\0'; ++p) {
+                               if (*p == '\\')
+                                       ++backslashes;
+                       }
+
+                       cgrout = calloc(sizeof(char), strlen(cgrpath) + backslashes + 1);
+                       ptrverify(cgrout, "Malloc failed for output cgroup path\n");
+                       char *p = cgrout, *p2 = cgrpath;
+                       while (*p2) {
+                               *p++ = *p2;
+                               if (*p2  == '\\')
+                                       *p++ = '\\'; // Double the backslash
+                               p2++;
+                       }
+                       *p = '\0'; // Null terminate the new string
+                       free(cgrpath);
+               }
                else
-                       cgrpath = "-";
+                       cgrout = "-";

                printf("{\"pid\": %d, "
                        "\"name\": \"(%.19s)\", "
@@ -1302,10 +1319,10 @@ json_print_PRC(char *hp, struct sstat *ss,
                        ps->cpu.nvcsw,
                        ps->cpu.nivcsw,
                        ps->cpu.sleepavg,
-                       cgrpath);
+                       cgrout);

                if (supportflags & CGROUPV2 && ps->gen.cgroupix != -1)
-                       free(cgrpath);
+                       free(cgrout);
        }

        printf("]");

Testing:

$ ./atop -J PRC 1 1 | sed -E 's/,/,\n/g' | jq | grep 'x2d'
      "cgroup": "/system.slice/system-serial\\x2dgetty.slice/serial-getty@ttyS0.service"
      "cgroup": "/system.slice/system-serial\\x2dgetty.slice/serial-getty@ttyS1.service"

Coding style & naming probably should be converted to adapt to project style. And escape routine should be done as a function, similar to parsable.c.