On Mon, Oct 8, 2012 at 6:52 PM, Lucas De Marchi
<lucas.demar...@profusion.mobi> wrote:
> 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)
Just made a quick test here and it works:
└ kmod ➤ ./tools/modprobe -c | grep testmodule
options testmodule testparam=1.5G
Lucas De Marchi
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
