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-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 15 Nov 2023 17:57:27 +0100
From: Petr Machata <petrm@...dia.com>
To: Stephen Hemminger <stephen@...workplumber.org>
CC: Petr Machata <petrm@...dia.com>, David Ahern <dsahern@...il.com>,
	<netdev@...r.kernel.org>, Patrisious Haddad <phaddad@...dia.com>
Subject: Re: [PATCH iproute2-next 3/3] lib: utils: Add
 parse_one_of_deprecated(), parse_on_off_deprecated()


Stephen Hemminger <stephen@...workplumber.org> writes:

> On Wed, 15 Nov 2023 16:31:59 +0100
> Petr Machata <petrm@...dia.com> wrote:
>
>> The functions parse_on_off() and parse_one_of() currently use matches() for
>> string comparison under the hood. This has some odd consequences. In
>> particular, "o" can be used as a shorthand for "off", which is not obvious,
>> because "o" is the prefix of both. By sheer luck, the end result actually
>> makes some sense: "on" means on, anything else means off (or errors out).
>> Similar issues are in principle also possible for parse_one_of() uses,
>> though currently this does not come up.
>
> This was probably a bug, I am open to breaking shorthand usage in this case.

There were uses of matches() for on/off parsing even before adding
parse_on_off(). The bug was converting _everyone_ to matches().

I figured you'd be against just s/matches/strcmp, but if you think it's
OK, I have no problem with that. Shorthanding on/off just makes no
sense to me, not even by mistake.

How about the parse_one_of() users? E.g. the disabled/check/strict for
macsec validate, I could see someone mistyping it as "disable", so now
it lives in some deployment script, or testing harness, or whatever.

Maybe do the warning thing in this case? And retire it a couple releases
down the line in favor of just accepting strcmp?

Powered by blists - more mailing lists