On Tuesday 01 February 2011 23:48, Denys Vlasenko wrote:
> On Tuesday 24 August 2010 06:01, Carmelo AMOROSO wrote:
> > If there are some spaces in the insmod command line, then
> > this will be splitted in single words as separate elements of
> > argv. It just needs to chain them together in the options string
> > passed to the sys_init_module syscall.
> >
> > Signed-off-by: Carmelo Amoroso <[email protected]>
> > ---
> > modutils/modutils.c | 3 +--
> > 1 files changed, 1 insertions(+), 2 deletions(-)
> >
> > diff --git a/modutils/modutils.c b/modutils/modutils.c
> > index 2608182..cc718db 100644
> > --- a/modutils/modutils.c
> > +++ b/modutils/modutils.c
> > @@ -71,8 +71,7 @@ char* FAST_FUNC parse_cmdline_module_options(char **argv)
> > optlen = 0;
> > while (*++argv) {
> > options = xrealloc(options, optlen + 2 + strlen(*argv) + 2);
> > - /* Spaces handled by "" pairs, but no way of escaping quotes */
> > - optlen += sprintf(options + optlen, (strchr(*argv, ' ') ?
> > "\"%s\" " : "%s "), *argv);
> > + optlen += sprintf(options + optlen, "%s ", *argv);
> > }
> > return options;
> > }
>
>
> Houston, we have a problem:
>
> On Tuesday 01 February 2011 23:25, Denys Vlasenko wrote:
> > On Tuesday 01 February 2011 19:48, Ralf Friedl wrote:
> > > Hi
> > >
> > > The commit
> > > http://git.buildroot.org/busybox/commit/?id=1396221d5a741ef8e1e8abca88836b341a3cab84
> > >
> > > breaks modprobe/insmod with spaces inside the parameters.
> > >
> > > When calling
> > > modprobe -v iptable_filter forward='1 2'
> > > the old version did
> > > init_module(0x499838, 5306, "\"forward=1 2\" ") = 0
> > > while the new version does
> > > init_module(0x491838, 5306, "forward=1 2 ") = -1 ENOENT (No such
> > > file or directory)
> > > And dmesg says
> > > iptable_filter: Unknown parameter `2'
> > >
> > > The kernel considers the '2' after the space to be the name of another
> > > parameter and not part of the value of forward. From the comments, this
> > > change seems intentional, but I don't see any advantage of this patch.
> > > Actually it seems obvious that the kernel can't recognize parameters
> > > with spaces after this patch.
> > > If it's just about code size, using the quotes always should also be
> > > possible.
> >
> > Hmm. You are right. Reverting.
> >
> > But even old behavior wasn't exactly correct. It was doing:
> >
> > init_module(0x8c8e1b8, 4744, "\"forward=1 2\" ") = -1 ENOENT (No such file
> > or directory)
> >
> >
> > whereas module-init-tools version 3.11.1 does:
> >
> > init_module(0xf6eab0, 4744, "forward=\"1 2\"") = -1 ENOENT (No such file or
> > directory)
> >
> > See? qoutes should be around value only.
Proposed fix:
function old new delta
parse_cmdline_module_options 102 157 +55
modprobe_main 657 662 +5
insmod_main 68 70 +2
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 3/0 up/down: 62/0) Total: 62 bytes
diff -ad -urpN busybox.2/modutils/insmod.c busybox.3/modutils/insmod.c
--- busybox.2/modutils/insmod.c 2011-01-31 05:50:35.000000000 +0100
+++ busybox.3/modutils/insmod.c 2011-02-01 23:45:56.000000000 +0100
@@ -59,7 +59,7 @@ int insmod_main(int argc UNUSED_PARAM, c
if (!filename)
bb_show_usage();
- rc = bb_init_module(filename, parse_cmdline_module_options(argv));
+ rc = bb_init_module(filename, parse_cmdline_module_options(argv,
/*quote_spaces:*/ 0));
if (rc)
bb_error_msg("can't insert '%s': %s", filename, moderror(rc));
diff -ad -urpN busybox.2/modutils/modprobe.c busybox.3/modutils/modprobe.c
--- busybox.2/modutils/modprobe.c 2011-01-31 05:50:35.000000000 +0100
+++ busybox.3/modutils/modprobe.c 2011-02-01 23:45:56.000000000 +0100
@@ -589,7 +589,7 @@ int modprobe_main(int argc UNUSED_PARAM,
/* First argument is module name, rest are parameters */
DBG("probing just module %s", *argv);
add_probe(argv[0]);
- G.cmdline_mopts = parse_cmdline_module_options(argv);
+ G.cmdline_mopts = parse_cmdline_module_options(argv,
/*quote_spaces:*/ 1);
}
/* Happens if all requested modules are already loaded */
diff -ad -urpN busybox.2/modutils/modutils.c busybox.3/modutils/modutils.c
--- busybox.2/modutils/modutils.c 2011-02-01 23:22:15.000000000 +0100
+++ busybox.3/modutils/modutils.c 2011-02-01 23:45:29.000000000 +0100
@@ -62,7 +62,7 @@ char* FAST_FUNC filename2modname(const c
return modname;
}
-char* FAST_FUNC parse_cmdline_module_options(char **argv)
+char* FAST_FUNC parse_cmdline_module_options(char **argv, int quote_spaces)
{
char *options;
int optlen;
@@ -70,12 +70,28 @@ char* FAST_FUNC parse_cmdline_module_opt
options = xzalloc(1);
optlen = 0;
while (*++argv) {
- options = xrealloc(options, optlen + 2 + strlen(*argv) + 2);
- /* Spaces handled by "" pairs, but no way of escaping quotes */
-//TODO: module-init-tools version 3.11.1 quotes only value:
-//it generates var="val with spaces", not "var=val with spaces"
-//(and it won't quote var *name* even if it has spaces)
- optlen += sprintf(options + optlen, (strchr(*argv, ' ') ?
"\"%s\" " : "%s "), *argv);
+ const char *fmt;
+ const char *var;
+ const char *val;
+
+ var = *argv;
+ options = xrealloc(options, optlen + 2 + strlen(var) + 2);
+ fmt = "%.*s%s ";
+ val = strchrnul(var, '=');
+ if (quote_spaces) {
+ /*
+ * modprobe (module-init-tools version 3.11.1) compat:
+ * quote only value:
+ * var="val with spaces", not "var=val with spaces"
+ * (note: var *name* is not checked for spaces!)
+ */
+ if (*val) { /* has var=val format. skip '=' */
+ val++;
+ if (strchr(val, ' '))
+ fmt = "%.*s\"%s\" ";
+ }
+ }
+ optlen += sprintf(options + optlen, fmt, (int)(val - var), var,
val);
}
return options;
}
diff -ad -urpN busybox.2/modutils/modutils.h busybox.3/modutils/modutils.h
--- busybox.2/modutils/modutils.h 2011-01-31 05:50:35.000000000 +0100
+++ busybox.3/modutils/modutils.h 2011-02-01 23:45:56.000000000 +0100
@@ -21,7 +21,7 @@ void replace(char *s, char what, char wi
char *replace_underscores(char *s) FAST_FUNC;
int string_to_llist(char *string, llist_t **llist, const char *delim)
FAST_FUNC;
char *filename2modname(const char *filename, char *modname) FAST_FUNC;
-char *parse_cmdline_module_options(char **argv) FAST_FUNC;
+char *parse_cmdline_module_options(char **argv, int quote_spaces) FAST_FUNC;
/* insmod for 2.4 and modprobe's options (insmod 2.6 has no options at all): */
#define INSMOD_OPTS \
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
