linux-kernel - What should we do with match

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-next>] [day] [month] [year] [list]

Message-ID: <20200204222547.GA21277@agluck-desk2.amr.corp.intel.com>
Date:   Tue, 4 Feb 2020 14:25:47 -0800
From:   "Luck, Tony" <tony.luck@...el.com>
To:     David Woodhouse <dwmw2@...radead.org>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Borislav Petkov <bp@...en8.de>, x86@...nel.org,
        linux-kernel@...r.kernel.org
Subject: What should we do with match_option()?

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