mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Julian Braha <julianbraha@gmail.com>
To: Dmitrii Tulnov <tulnov.dl@gmail.com>,
	Nathan Chancellor <nathan@kernel.org>,
	Nicolas Schier <nsc@kernel.org>
Cc: Peter Korsgaard <jacmet@uclibc.org>,
	"Yann E. MORIN" <yann.morin.1998@free.fr>,
	linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] kconfig: reject malformed KCONFIG_PROBABILITY values
Date: Tue, 8 Sep 2026 20:24:34 +0100	[thread overview]
Message-ID: <ce0786a7-e48d-4d7b-b601-86a598bb03d1@gmail.com> (raw)
In-Reply-To: <20260907194536.37-1-tulnov.dl@gmail.com>

Hi Dmitrii,

I see that this is your first patch submission. Welcome!
And thank you for reporting this.

On 9/7/26 20:45, Dmitrii Tulnov wrote:
> randconfig checks the numeric range of each probability but does not
> validate where strtol() stops. For example, KCONFIG_PROBABILITY=50% is
> accepted as 50:0:0: the '%' is parsed twice as zero. This silently sets
> both tristate y/m probabilities to zero, reducing the coverage of random
> configuration builds. 

It would be good to note here that 'KCONFIG_PROBABILITY=50' (without the
'%') should be 25:25:50 instead.

> Empty fields and extra fields are also accepted.

Right, in these cases, I think it's reasonable to assume the user made a
typo or other oversight.

> 
> Require each field to contain an integer followed by the end of the
> string or a colon introducing another field, with at most three fields.
> Preserve the leading whitespace and optional sign accepted by strtol().

I disagree with allowing the leading whitespace and sign. The fact that
it's allowed input for strtol() is incidental to us. The intended input
format for Kconfig is:

XX:XX:XX

not:

  XX: +XX:    -XX

> Keep the strtol() result as long until the range check so that narrowing
> to int cannot turn an out-of-range value into a valid probability.
> 
> Add regression tests for malformed and out-of-range values, the supported
> probability formats, and the documented empty-value default.
> 
> Fixes: e43956e60769 ("kconfig: implement KCONFIG_PROBABILITY for randconfig")
> Assisted-by: LLM
> Signed-off-by: Dmitrii Tulnov <tulnov.dl@gmail.com>
> ---
> Validation on kbuild-next, x86_64, GCC 13.3.0:
> - make testconfig with HOSTCFLAGS=-Werror: 58 passed. With the original
>   conf binary: 18 failed, 40 passed; all failures are new regression tests.
> - ASan/UBSan at -O1, with leak detection disabled: the same 58 tests passed.
> - Both builds passed 1,635 input cases and 440 comparisons with the original
>   conf using valid inputs and fixed seeds. Malformed inputs preserved an
>   existing .config, including when KCONFIG_ALLCONFIG was set.
> - make defconfig, allnoconfig, allmodconfig and valid randconfig passed.
>   KCONFIG_PROBABILITY=50% changed from success to the expected error.
> - No vmlinux build or boot test; this changes the host configuration tool.
>   A 32-bit host build was unavailable because multilib headers were missing.
> 
> An AI coding assistant helped find the issue, prepare the fix, description
> and tests, and run validation. The requested task was to find and fix a
> useful, reproducible Linux bug suitable for a first contribution.
> 
>  scripts/kconfig/conf.c                        |  9 +++-
>  .../tests/randconfig_probability/Kconfig      | 12 +++++
>  .../tests/randconfig_probability/__init__.py  | 54 +++++++++++++++++++
>  3 files changed, 74 insertions(+), 1 deletion(-)
>  create mode 100644 scripts/kconfig/tests/randconfig_probability/Kconfig
>  create mode 100644 scripts/kconfig/tests/randconfig_probability/__init__.py
> 
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index fe8ba09b0..fa5dae74e 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -191,7 +191,14 @@ static void conf_set_all_new_symbols(enum conf_def_mode mode)
>  		n = 0;
>  		while (env && *env) {
>  			char *endp;
> -			int tmp = strtol(env, &endp, 10);
> +			long tmp = strtol(env, &endp, 10);
> +
> +			if (endp == env || (*endp && *endp != ':') ||
> +			    (*endp == ':' && (!endp[1] || n == 2))) {
> +				errno = EINVAL;
> +				perror("KCONFIG_PROBABILITY");
> +				exit(1);

Unfortunately, sometimes users depend on unintended behavior. So I would
prefer to make this a warning for now, and then promote this to an error
later assuming nobody complains.

- Julian Braha

  reply	other threads:[~2026-09-08 19:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-07 19:45 Dmitrii Tulnov
2026-09-08 19:24 ` Julian Braha [this message]
2026-09-08 20:35   ` Dmitrii Tulnov
2026-09-08 20:36   ` [PATCH v2] kconfig: warn about " Dmitrii Tulnov
2026-09-10  0:05     ` Julian Braha
2026-09-10  9:13       ` Dmitrii Tulnov
2026-09-10 11:25         ` Julian Braha
2026-09-10 12:04           ` [PATCH v3] " Dmitrii Tulnov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ce0786a7-e48d-4d7b-b601-86a598bb03d1@gmail.com \
    --to=julianbraha@gmail.com \
    --cc=jacmet@uclibc.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nathan@kernel.org \
    --cc=nsc@kernel.org \
    --cc=tulnov.dl@gmail.com \
    --cc=yann.morin.1998@free.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®