What should we do with match_option()?

What should we do with match_option()?

[Luck, Tony]

Back at the beginning of 2018 David Woodhouse added the inline
function match_option() to aid is parsing boot arguments:

da285121560e ("x86/spectre: Add boot time option to select Spectre v2 mitigation")

More recently PeterZ used match_option() in some pseudo-code to help
get the split-lock patches un-jammed.  I cleaned that up a bit and
the patch is now sitting in TIP ready for the next merge window.

But Boris noticed that I'd copy/pasted the inline function defintion,
and I promised to look at resolving the duplication.

My first instinct was to just delete both instances from ".c" files
and move it to <linux/string.h>. But net/netfilter/xt_dccp.c has its
own function with this name (that does something different).

So I looked a bit more closely at what it actually does ... and now
I'm not really sure what problem it is solving.

The issue seems to be that cmdline_find_option() might truncate the
value of the option string to fit in the user supplied buffer. If that
happens, the value in the buffer is guaranteed NUL terminated and
cmdline_find_option() returns the length of the full string.

match_option() checks to see if that return value matches the length
of the option being checked, and fails if it doesn't match. Which
prevents the truncated string from giving a false match against the
option string being checked.

But this seems to be a belt, braces (USA=suspenders) and stapling the
waistband of trousers (USA=pants) to your body approach.

If the user supplies a large enough buffer to cmdline_find_option()
for any of the legal options Then the resulting "arg" will not be
truncated for anything legal. So we should be able to just use
"strcmp()" to see which of the options is matched.

So should we promote match_option() to <linux/string.h>? Or
drop it and just use strcmp() instead?

-Tony

Re: What should we do with match_option()?

[Borislav Petkov]

On Tue, Feb 04, 2020 at 02:25:47PM -0800, Luck, Tony wrote:
> But this seems to be a belt, braces (USA=suspenders) and stapling the
> waistband of trousers (USA=pants) to your body approach.

Haha.

> If the user supplies a large enough buffer to cmdline_find_option()
> for any of the legal options Then the resulting "arg" will not be
> truncated for anything legal. So we should be able to just use
> "strcmp()" to see which of the options is matched.
> 
> So should we promote match_option() to <linux/string.h>? Or
> drop it and just use strcmp() instead?

Makes sense to me: cmdline_find_option() will make sure the string is
NULL-terminated if the buffer is smaller than the option so strcmp()
should not go off into the weeds. The worst that can happen, AFAICT,
is the option not matching but that should be picked up pretty soon in
testing. (I'm assuming we all test our code before sending :-))).

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette