Messages in this thread | | | From | Rusty Russell <> | Subject | Re: [PATCH] [RFC] Taint the kernel for unsafe module options | Date | Fri, 07 Mar 2014 13:58:21 +1030 |
| |
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.
| |