On Sun, Oct 7, 2012 at 2:35 PM, Marco d'Itri <m...@linux.it> wrote: > Any objections? > > ----- Forwarded message from "Selim T. Erdogan" <se...@alumni.cs.utexas.edu> > ----- > > From: "Selim T. Erdogan" <se...@alumni.cs.utexas.edu> > To: Debian Bug Tracking System <sub...@bugs.debian.org> > Subject: Bug#689872: libkmod2: module parameters on kernel command line parsed > wrong > > Package: libkmod2 > Version: 9-2 > Severity: important > Tags: d-i patch > > This problem only affects module parameters given on the kernel command > line at boot time which include a '.' as part of their value. For > example "libata.force=1.5Gbps". The bug causes such values to > overwrite the parameter, so this would get interpreted as parameter > "5Gbps" with no value. ("force" gets lost.) > > To reproduce this safely, add "testmodule.testparam=1.5G" to the end of > the kernel command line at boot time. Then: > > $ /sbin/modprobe -c |grep 5G > options testmodule 5G > > This problem prevented me from using the wheezy beta 2 debian-installer > on a machine that needed libata.force=1.5Gbps to work properly with my > new SSD. See https://lists.debian.org/debian-boot/2012/10/msg00049.html > > I'm including a very simple at the end of the report that fixes the > problem. Feel free to modify/edit as necessary. With this patch: > > $ /sbin/modprobe -c |grep 5G > options testmodule testparam=1.5G > > > -- System Information: > Debian Release: wheezy/sid > APT prefers unstable > APT policy: (500, 'unstable'), (1, 'experimental') > Architecture: i386 (i686) > > Kernel: Linux 3.2.0-3-686-pae (SMP w/1 CPU core) > Locale: LANG=tr_TR.UTF-8, LC_CTYPE=tr_TR.UTF-8 (charmap=UTF-8) > Shell: /bin/sh linked to /bin/dash > > Versions of packages libkmod2 depends on: > ii libc6 2.13-35 > ii multiarch-support 2.13-35 > > libkmod2 recommends no packages. > > libkmod2 suggests no packages. > > -- no debconf information > > *** kcmdline-option-parsing > Description: Fixes parsing of module parameters on the kernel command > line. Before this patch, if the parameter value had a '.' in it, that > would break things. > Author: Selim T. Erdoğan <se...@alumni.cs.utexas.edu> > > --- kmod-9.orig/libkmod/libkmod-config.c > +++ kmod-9/libkmod/libkmod-config.c > @@ -542,6 +542,7 @@ static int kmod_config_parse_kcmdline(st > char buf[KCMD_LINE_SIZE]; > int fd, err; > char *p, *modname, *param = NULL, *value = NULL; > + int in_value_text = 0; > > fd = open("/proc/cmdline", O_RDONLY|O_CLOEXEC); > if (fd < 0) { > @@ -564,15 +565,20 @@ static int kmod_config_parse_kcmdline(st > *p = '\0'; > kcmdline_parse_result(config, modname, param, value); > param = value = NULL; > + in_value_text = 0; > modname = p + 1; > break; > case '.': > - *p = '\0'; > - param = p + 1; > - break; > + if (in_value_text == 0) { > + *p = '\0'; > + param = p + 1; > + break; > + }
this fall-through here doesn't seem intentional > case '=': > - if (param != NULL) > + if (param != NULL) { > value = p + 1; > + in_value_text=1; > + } > break; > } > } > Why adding another var? Isn't something like below (whitespace damaged) sufficient? Lucas De Marchi diff --git a/libkmod/libkmod-config.c b/libkmod/libkmod-config.c index 70044f0..398468e 100644 --- a/libkmod/libkmod-config.c +++ b/libkmod/libkmod-config.c @@ -567,8 +567,10 @@ static int kmod_config_parse_kcmdline(struct kmod_config *config) modname = p + 1; break; case '.': - *p = '\0'; - param = p + 1; + if (param == NULL) { + *p = '\0'; + param = p + 1; + } break; case '=': if (param != NULL) -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org