Messages in this thread | | | Date | Mon, 14 Feb 2022 17:19:49 -0800 | Subject | Re: Wrong __setup() callbacks return values and /init's environment pollution | From | Randy Dunlap <> |
| |
Hi--
I have had this in my todo bucket for too long now...
On 12/25/20 09:20, Igor Zhbanov wrote: > It seems that nobody knows how to write __setup() callback functions for > parsing the command line parameters. And there are no documentation or > comments about best practices. > > Despite being declared obsolete __setup() is used about 435 times in the > kernel and 51 of them (~11.2%) are erroneous in the sense of returning > incorrect value (0) resulting in the /init process environment pollution. > > Initially it was mentioned that the callback function should return 1 when > the parameter value is parsed and consumed successfully, or return 0 to > keep the unparsed option as init's environment variable. > > But there are no comments or documentation about it, so developers often > always returning 0 (as it is typically expected in other kernel functions) > or returning -EINVAL on parse error. All these cases resulting in init's > environment pollution (which is limited only to 32 variables in the config). > > Even the original usage when 0 is returned on the parse error is questionable > now. There are no (or not so many) parameter names clashes between different > kernel subsystems, as well as there not so many parameters to be passed to > init/systemd that could be interpreted as the kernel parameters. So if a > kernel module sure that this is its own parameter, may be it would be better > to always return 1 and consume it, even it is malformed. > > Also there are no recommendations on whether to print a warning when > incorrect parameter is passed. Some of the functions print a warning on > incorrect values; some are silently proceeding with the default values. > Some are even calling panic() on incorrect parameters (e.g. > setup_time_travel() -> time_travel_connect_external()). So there is no > consistency on what behavior for handling incorrect parameters values are > recommended. > > I've tried to categorize all of the __setup() usages. And here is the zoo: > > Functions always returning 1 (right): > (About 324)
[cuts]
> > Functions always returning 0 (wrong):
> > Functions returning 1 on error, 0 on success (wrong):
> > Functions returning -errrno (-EINVAL mostly) on error, 0 on success (wrong):
> > Functions returning -EINVAL on error, 1 on success (wrong):
> > Functions that don't have the return statement at all (wrong):
> > So it would be better to document at least how a __setup() callback handler > should behave regarding the return value, whether to print a warning on parse > error. And the functions not returning 0 on error and 1 on success must be > fixed.
Then how about starting with some documentation? Can you send some?
> Also to avoid init's processes pollution may be it is even better to simply > always returning 1 even on parse errors, because parameters names collisions > are very infrequent.
thanks. -- ~Randy
| |