micctrl crash

[Kazutomo_Y_]

micctrl crashes if I change OSimage in micX.conf on MPSS 3.2. I traced it and found a problem.   I checked the MPSS-3.2.1 source code and looks this problem still there. See below for a fix.

- kaz

--- mpss-daemon-3.2/libmpssconfig/mpssconfig.c.bak    2014-04-24 01:01:47.097544049 -0500
+++ mpss-daemon-3.2/libmpssconfig/mpssconfig.c    2014-04-24 01:02:01.256545552 -0500
@@ -205,7 +205,7 @@
         return 0;
     }
 
-    if ((mic->config.boot.systemmap = malloc(strlen(map)) + 1) != NULL) {
+    if ((mic->config.boot.systemmap = malloc(strlen(map)+ 1)) != NULL) {
         strcpy(mic->config.boot.systemmap, map);
         add_perr(perrs, PINFO, "%s: [Parse] OS System map %s", mic->name, map);
     }

[Loc_N_Intel]

Hi Kazutomo,

I tried to reproduce the problem you reported but I didn't see it. Here is what I did on my system with MPSS 3.2.1 (note that bzImage-2.6.38+mpss3.2-knightscorner and System.map-2.6.38+mpss3.2-kgnightscorner are currently existing files under /usr/share/mpss/boot)

% micinfo

% cd /usr/share/mpss/boot

% cp bzImage-2.6.38+mpss3.2-knightscorner bzImage-test

% micctrl --osimage=/usr/share/mpss/boot/bzImage-test --sysmap=/usr/share/mpss/boot/System.map-knightscorner mic0

% service mpss stop

% service mpss start

MPSS comes up properly, mic0 booted with bzImage-test.

Would you like to describe how did you find the issue? Thank you.

[Kazutomo_Y_]

Hi Loc-nguyen,

The code probably works without segv by luck. It depends on the string length (technically of the system map).

Please take a look at the source code of  mpss-daemon-3.2/libmpssconfig/mpssconfig.c  The line number 208 in a copy of the mpss source code I have

    if ((mic->config.boot.systemmap = malloc(strlen(map)) + 1) != NULL) {

This malloac only allocates the size strlen(map) and adds 1 to the malloc result.  The following strcpy is likely to trigger a data corruption.

- kaz

[Loc_N_Intel]

Hi Kazutomo,

I also tried with sysmap in micctrl but still couldn't reproduce the problem. However, I agree the code needs to be changed. I did file a bug report, (ID: 4868934) to report this issue..

Thank you..