diff mbox series

kconfig: Sanitize make randconfig generated .config

Message ID 20200122100344.50751-1-vincenzo.frascino@arm.com (mailing list archive)
State New, archived
Headers show
Series kconfig: Sanitize make randconfig generated .config | expand

Commit Message

Vincenzo Frascino Jan. 22, 2020, 10:03 a.m. UTC
"make randconfig" calculates the probability of a tristate option (yes,
mod, no) based on srand()/rand() and can be fed with a seed.
At the last step of randconfig some option are chosen randomly and their
tristate set based on similar mechanism.
After this passage the resulting .config is not sanitized, hence it
might result in an inconsistent set of options being selected.

This was noticed on arm64 using KCONFIG_SEED=0x40C5E904. During
randomize_choice_values() CONFIG_BIG_ENDIAN is enabled. Since CONFIG_EFI
was enabled at a previous step, and depends on !CONFIG_BIG_ENDIAN the
resulting .config is inconsistent.

Fix the issue making sure that randconfig sanitizes the generated
.config as a last step.

Cc: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 scripts/kconfig/conf.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Masahiro Yamada Feb. 1, 2020, 3:12 a.m. UTC | #1
Hi.


On Wed, Jan 22, 2020 at 7:03 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> "make randconfig" calculates the probability of a tristate option (yes,
> mod, no) based on srand()/rand() and can be fed with a seed.
> At the last step of randconfig some option are chosen randomly and their
> tristate set based on similar mechanism.
> After this passage the resulting .config is not sanitized, hence it
> might result in an inconsistent set of options being selected.
>
> This was noticed on arm64 using KCONFIG_SEED=0x40C5E904. During
> randomize_choice_values() CONFIG_BIG_ENDIAN is enabled. Since CONFIG_EFI
> was enabled at a previous step, and depends on !CONFIG_BIG_ENDIAN the
> resulting .config is inconsistent.
>
> Fix the issue making sure that randconfig sanitizes the generated
> .config as a last step.
>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> ---
>  scripts/kconfig/conf.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 1f89bf1558ce..c0fcaa4e9762 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -654,6 +654,11 @@ int main(int ac, char **av)
>         case randconfig:
>                 /* Really nothing to do in this loop */
>                 while (conf_set_all_new_symbols(def_random)) ;
> +               /*
> +                * .config at this point might contain
> +                * incompatible options. Sanitize it.
> +                */
> +               sym_clear_all_valid();

Thanks for the report, but clearing
all the valid flags is a big hammer.
I do not think it is a proper fix.


I checked the code, and I noticed the root cause of
this bug.

I will send a different patch later.




>                 break;
>         case defconfig:
>                 conf_set_all_new_symbols(def_default);
> --
> 2.25.0
>
Masahiro Yamada Feb. 1, 2020, 5:15 a.m. UTC | #2
On Sat, Feb 1, 2020 at 12:12 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Hi.
>
>
> On Wed, Jan 22, 2020 at 7:03 PM Vincenzo Frascino
> <vincenzo.frascino@arm.com> wrote:
> >
> > "make randconfig" calculates the probability of a tristate option (yes,
> > mod, no) based on srand()/rand() and can be fed with a seed.
> > At the last step of randconfig some option are chosen randomly and their
> > tristate set based on similar mechanism.
> > After this passage the resulting .config is not sanitized, hence it
> > might result in an inconsistent set of options being selected.
> >
> > This was noticed on arm64 using KCONFIG_SEED=0x40C5E904. During
> > randomize_choice_values() CONFIG_BIG_ENDIAN is enabled. Since CONFIG_EFI
> > was enabled at a previous step, and depends on !CONFIG_BIG_ENDIAN the
> > resulting .config is inconsistent.
> >
> > Fix the issue making sure that randconfig sanitizes the generated
> > .config as a last step.
> >
> > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> > ---
> >  scripts/kconfig/conf.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> > index 1f89bf1558ce..c0fcaa4e9762 100644
> > --- a/scripts/kconfig/conf.c
> > +++ b/scripts/kconfig/conf.c
> > @@ -654,6 +654,11 @@ int main(int ac, char **av)
> >         case randconfig:
> >                 /* Really nothing to do in this loop */
> >                 while (conf_set_all_new_symbols(def_random)) ;
> > +               /*
> > +                * .config at this point might contain
> > +                * incompatible options. Sanitize it.
> > +                */
> > +               sym_clear_all_valid();
>
> Thanks for the report, but clearing
> all the valid flags is a big hammer.
> I do not think it is a proper fix.
>
>
> I checked the code, and I noticed the root cause of
> this bug.
>
> I will send a different patch later.


I think this is a more correct fix-up:
https://patchwork.kernel.org/patch/11360945/
Vincenzo Frascino Feb. 7, 2020, 10:40 a.m. UTC | #3
On 01/02/2020 03:12, Masahiro Yamada wrote:
> Hi.
> 
> 
> On Wed, Jan 22, 2020 at 7:03 PM Vincenzo Frascino
> <vincenzo.frascino@arm.com> wrote:
>>
>> "make randconfig" calculates the probability of a tristate option (yes,
>> mod, no) based on srand()/rand() and can be fed with a seed.
>> At the last step of randconfig some option are chosen randomly and their
>> tristate set based on similar mechanism.
>> After this passage the resulting .config is not sanitized, hence it
>> might result in an inconsistent set of options being selected.
>>
>> This was noticed on arm64 using KCONFIG_SEED=0x40C5E904. During
>> randomize_choice_values() CONFIG_BIG_ENDIAN is enabled. Since CONFIG_EFI
>> was enabled at a previous step, and depends on !CONFIG_BIG_ENDIAN the
>> resulting .config is inconsistent.
>>
>> Fix the issue making sure that randconfig sanitizes the generated
>> .config as a last step.
>>
>> Cc: Masahiro Yamada <masahiroy@kernel.org>
>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>> ---
>>  scripts/kconfig/conf.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>> index 1f89bf1558ce..c0fcaa4e9762 100644
>> --- a/scripts/kconfig/conf.c
>> +++ b/scripts/kconfig/conf.c
>> @@ -654,6 +654,11 @@ int main(int ac, char **av)
>>         case randconfig:
>>                 /* Really nothing to do in this loop */
>>                 while (conf_set_all_new_symbols(def_random)) ;
>> +               /*
>> +                * .config at this point might contain
>> +                * incompatible options. Sanitize it.
>> +                */
>> +               sym_clear_all_valid();
> 
> Thanks for the report, but clearing
> all the valid flags is a big hammer.
> I do not think it is a proper fix.
> 
>

Ok, it is fine by me to find a better solution in this case. Not being on the
critical path of execution it did not look like to me very heavy but my
experience with the tool code is limited :)

> I checked the code, and I noticed the root cause of
> this bug.
> 
> I will send a different patch later.
> 
> 
> 
> 
>>                 break;
>>         case defconfig:
>>                 conf_set_all_new_symbols(def_default);
>> --
>> 2.25.0
>>
> 
>
diff mbox series

Patch

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 1f89bf1558ce..c0fcaa4e9762 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -654,6 +654,11 @@  int main(int ac, char **av)
 	case randconfig:
 		/* Really nothing to do in this loop */
 		while (conf_set_all_new_symbols(def_random)) ;
+		/*
+		 * .config at this point might contain
+		 * incompatible options. Sanitize it.
+		 */
+		sym_clear_all_valid();
 		break;
 	case defconfig:
 		conf_set_all_new_symbols(def_default);