Bug 3223 - insmod/modprobe incorrectly parses several parameters in kernel 2.4
Summary: insmod/modprobe incorrectly parses several parameters in kernel 2.4
Status: RESOLVED FIXED
Alias: None
Product: Busybox
Classification: Unclassified
Component: Other (show other bugs)
Version: 1.18.x
Hardware: All Linux
: P3 major
Target Milestone: ---
Assignee: unassigned
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-10 18:57 UTC by Leonid
Modified: 2011-09-11 17:40 UTC (History)
1 user (show)

See Also:
Host:
Target: linux 2.4
Build:


Attachments
Proposed patch (418 bytes, patch)
2011-02-10 18:57 UTC, Leonid
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Leonid 2011-02-10 18:57:31 UTC
Created attachment 2953 [details]
Proposed patch

insmod/modprobe incorrectly parses several char & string parameters in kernel 2.4

Sample output:

# insmod netconsole.o netconsole=@/,@192.168.1.100/
insmod: parameter netconsole requires at least 2 arguments

# modinfo netconsole.o 
filename:       netconsole.o
parm_desc_netconsole: netconsole=[src-port]@[src-ip]/[dev],[tgt-port]@<tgt-ip>/[tgt-macaddr]

parm_netconsole:2s
license:        GPL
description:    Network console driver
author:         Maintainer: Herbert P�tzl <herbert@13thfloor.at>
kernel_version: 2.4.37.11


This happens due to "Parse parameter values" loop in modutils-24.c: new_process_module_arguments() wants to recheck ',' delimiter at the end of switch (*pinfo), but it already overwritten by '\0' symbol in code above for 's' & 'c' cases.

Patch attached solves problem for me.
Comment 1 Denys Vlasenko 2011-02-13 03:05:49 UTC
-                       p = skip_whitespace(p);
-                       if (*p != ',')
-                               break;
+                       if (*p != '\0') {
+                               p = skip_whitespace(p);
+                               if (*p != ',')
+                                       break;
+                       }
                        p = skip_whitespace(p + 1);

And if *p == '\0' because we genuinely reached terminating NUL, not because we replaces ',' with '\0', then what p = skip_whitespace(p + 1) do?
Comment 2 Denys Vlasenko 2011-02-13 03:08:42 UTC
I propose to save/restore the character, like this:

                                char sv_ch = p[len];
                                p[len] = '\0';
                                obj_string_patch(f, sym->secidx,
                                                 loc - contents, p);
                                loc += tgt_sizeof_char_p;
                                p += len;
                                *p = sv_ch;
Comment 3 Leonid 2011-02-13 17:57:24 UTC
You are right, my patch breaks some checks.

Anyway, bug fixed & can be closed.
Comment 4 Denys Vlasenko 2011-09-11 17:40:09 UTC
Fixed in 1.19.x