On Sun, Oct 7, 2012 at 2:35 PM, Marco d'Itri <[email protected]> wrote:
> Any objections?
>
> ----- Forwarded message from "Selim T. Erdogan" <[email protected]>
> -----
>
> From: "Selim T. Erdogan" <[email protected]>
> To: Debian Bug Tracking System <[email protected]>
> 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 <[email protected]>
>
> --- 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 [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]
