Re: [PATCH] [RFC] Taint the kernel for unsafe module options

[Rusty Russell <rusty@rustcorp.com.au>]

Daniel Vetter <daniel@ffwll.ch> writes:
> On Thu, Mar 06, 2014 at 11:19:54AM +1030, Rusty Russell wrote:
>> Daniel Vetter <daniel.vetter@ffwll.ch> writes:
>> > Users just love to set random piles of options since surely enabling
>> > all the experimental stuff helps. Later on we get bug reports because
>> > it all fell apart.
>> >
>> > Even more fun when it's labelled a regression when some change only
>> > just made the feature possible (e.g. stolen memory fixes suddenly
>> > making fbc possible).
>> >
>> > Make it clear that users are playing with fire here. In drm/i915 all
>> > these options follow the same pattern of using -1 as the per-machine
>> > default, and any other value being used for force the parameter.
>> >
>> > Adding a pile of cc's to solicit input and figure out whether this
>> > would be generally useful - this quick rfc is just for drm/i915.
>> 
>> If this is a good idea, you can write a macro module_param_unsafe_named
>> which is a general wrapper.
>
> For this to work I need to somehow store the safe default value somewhere.
> since with bools or strings there really isn't such a thing, even less
> than with integers where my fairly abitrary -1 choice is already
> restricting. But I don't have a good idea how to do that, since creating a
> local static struct in the macro to store the default + the pointer to the
> storage location feels a bit ugly.

I was thinking that if use the parameter, they get marked unsafe.  If
they use it to set it to the default, Don't Do That.

>> > -module_param_named(modeset, i915.modeset, int, 0400);
>> 
>> Wait, WTF?  Why do you prefix i915 here manually?  That means that
>> the commandline parameter would be "i915.i915.modeset=" and the
>> module parameter would be "i915.modeset="...
>
> Nope, this is the named macro. The name of the param is the first
> parameter to the macro "modeset", "i915.modeset" is just the variable
> it'll get stored in. We've specifically switched to the _named version to
> avoid ugly i915.i915* paramters ;-)

Oh, oops, my bad reading!  I'll shut up now :)

Thanks,
Rusty.