linux-kernel - Re: Problems with string (charp) module parameters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6dc076840910211913l1cf12e82tbb9151da78e41422@mail.gmail.com>
Date:	Thu, 22 Oct 2009 04:13:36 +0200
From:	Takashi Iwai <takashi.iwai@...il.com>
To:	Rusty Russell <rusty@...tcorp.com.au>
Cc:	Takashi Iwai <tiwai@...e.de>, linux-kernel@...r.kernel.org
Subject: Re: Problems with string (charp) module parameters

Hi Rusty,

(sending from gmail address since VPN doesn't work here in hotel...)

On Wed, Oct 21, 2009 at 3:11 PM, Rusty Russell <rusty@...tcorp.com.au> wrote:
> On Tue, 13 Oct 2009 09:07:46 pm Takashi Iwai wrote:
>> * The handling of parameter array is pretty buggy now.
>>   kp->perm and kp->flags aren't properly initialized in
>>   param_array().  Thus, you might call kfree() with invalid pointers,
>>   or pass a wrong type for bool.
>
> Yes, an array of charp isn't going to work.  Erk, I switched one bug for
> another :(
>
>> So, the situation looks messy right now, not only about the section
>> issue.  If we allow kmalloc of each parameter array element, the flag
>> must be associated to each element, not a global one to the array.
>>
>> Thoughts?
>
> Yes, that's hard.  There's only one place which currently has a writable
> array parameter: drivers/usb/atm/ueagle-atm.c, and it's root only.
>
> OK, for 2.6.32, we remove the const.  In the longer term, I'm reworking how
> this is done entirely.

As far as I checked, removing only const doesn't suffice on x86.
The problem is rather the __param section assignment.
We'd need to get rid of that, too, if we keep the code in the current way.

Anyway, I've thought of this for a while, but couldn't find any smart solution.
Judging from the current usage pattern, I think we should rather take
a simpler solution:
don't allow sysfs-write for charp type but use string type for
writable parameters.

It's not only for avoiding the mess to separate static and kmalloc
strings but also for
avoiding races between the referrer and the sysfs-write of char
pointer.  (In general, we
have no lock for parameters.)

As you pointed out, there are no many users of writable charp parameters.
So, replacing is easy task.  In that way, we can keep struct
kernel_parameter as const
gracefully without hustling any big code change.

thanks,

Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/