| 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
| ||
|
Message-ID: <20100811195131.GA24193@merkur.ravnborg.org>
Date: Wed, 11 Aug 2010 21:51:32 +0200
From: Sam Ravnborg <sam@...nborg.org>
To: Arve Hj?nnev?g <arve@...roid.com>
Cc: Michal Marek <mmarek@...e.cz>,
Linus Torvalds <torvalds@...ux-foundation.org>,
JBeulich@...ell.com, aris@...hat.com, catalin.marinas@....com,
jacmet@...site.dk, justinmattock@...il.com, lizf@...fujitsu.com,
ulfalizer.lkml@...il.com, zippel@...ux-m68k.org,
linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [GIT] kbuild: kconfig changes
On Fri, Aug 06, 2010 at 09:43:24PM -0700, Arve Hj?nnev?g wrote:
> On Fri, Aug 6, 2010 at 9:01 PM, Sam Ravnborg <sam@...nborg.org> wrote:
> >>
> >> This change prevents some the minimal defconfig options from working.
> >> Specifically our usb gadget drivers do not get set.
> >
> > Can you help me reproduce this?
> >
> > I have found an issue with choice values in combination with
> > tristate logic that fails. I hope this is something similar.
> >
>
> It is probably the same problem. The gadget driver that was not set is
> not buildable as a module (it is not in the mainline kernel). If I
> select another gadget driver instead it just gets changed to build as
> a module instead.
>
> If you create a file, arch/arm/configs/test_defconfig with the following:
> CONFIG_MODULES=y
> CONFIG_USB_GADGET=y
> CONFIG_USB_MASS_STORAGE=y
>
> then "make test_defconfig" results in .config having:
> CONFIG_USB_MASS_STORAGE=m
>
> (at least if you are set up to compile for arm)
Arve, can you please try the attached patch and check if this
fixes the issue you see.
At least I confirmed that it fixed the minimal test-case I used.
Note: the patch chenges how we interpret a minimal config,
it does not change the content of a minimal config.
Unfortunately the bug I had discovered is another bug.
If I do:
make ARCH=avr32 atngw100_defconfig
make ARCH=avr32 savedefconfig
cp defconfig arch/avr32/configs/atngw100_defconfig
make ARCH=avr32 atngw100_defconfig
diff -u .config .config.old
then the diff shows an unexpected difference.
I will continue of that issue.
This is mainly to inform you that we have at least one
additional issue with "savedefconfig".
Sam
>From 6f36cdab76c4ee26bc39c38a2895cac5166e253b Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <sam@...nborg.org>
Date: Wed, 11 Aug 2010 21:40:23 +0200
Subject: [PATCH] kconfig: fix tristate choice with minimal config
If a minimal config did not specify the value
of all choice values, the resulting configuration
could have wrong values.
Consider following example:
config M
def_bool y
option modules
choice
prompt "choice list"
config A
tristate "a"
config B
tristate "b"
endchoice
With a defconfig like this:
CONFIG_M=y
CONFIG_A=y
The resulting configuration would have
CONFIG_A=m
which was unexpected.
The problem was not not all choice values were set and thus
kconfig calculated a wrong value.
The fix is to set all choice values when we
read a defconfig files.
conf_set_all_new_symbols() is refactored such that
random choice values are now handled by a dedicated function.
And new choice values are set by set_all_choice_values().
This was not the minimal fix, but the fix that resulted
in the most readable code.
Signed-off-by: Sam Ravnborg <sam@...nborg.org>
Cc: Arve Hj?nnev?g <arve@...roid.com>
---
scripts/kconfig/confdata.c | 102 +++++++++++++++++++++++++++++---------------
1 files changed, 67 insertions(+), 35 deletions(-)
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index f81f263..23d4fa6 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -919,13 +919,73 @@ void conf_set_changed_callback(void (*fn)(void))
conf_changed_callback = fn;
}
+static void randomize_choice_values(struct symbol *csym)
+{
+ struct property *prop;
+ struct symbol *sym;
+ struct expr *e;
+ int cnt, def;
-void conf_set_all_new_symbols(enum conf_def_mode mode)
+ /*
+ * If choice is mod then we may have more items slected
+ * and if no then no-one.
+ * In both cases stop.
+ */
+ if (csym->curr.tri != yes)
+ return;
+
+ prop = sym_get_choice_prop(csym);
+
+ /* count entries in choice block */
+ cnt = 0;
+ expr_list_for_each_sym(prop->expr, e, sym)
+ cnt++;
+
+ /*
+ * find a random value and set it to yes,
+ * set the rest to no so we have only one set
+ */
+ def = (rand() % cnt);
+
+ cnt = 0;
+ expr_list_for_each_sym(prop->expr, e, sym) {
+ if (def == cnt++) {
+ sym->def[S_DEF_USER].tri = yes;
+ csym->def[S_DEF_USER].val = sym;
+ }
+ else {
+ sym->def[S_DEF_USER].tri = no;
+ }
+ }
+ csym->flags |= SYMBOL_DEF_USER;
+ /* clear VALID to get value calculated */
+ csym->flags &= ~(SYMBOL_VALID);
+}
+
+static void set_all_choice_values(struct symbol *csym)
{
- struct symbol *sym, *csym;
struct property *prop;
+ struct symbol *sym;
struct expr *e;
- int i, cnt, def;
+
+ prop = sym_get_choice_prop(csym);
+
+ /*
+ * Set all non-assinged choice values to no
+ */
+ expr_list_for_each_sym(prop->expr, e, sym) {
+ if (!sym_has_value(sym))
+ sym->def[S_DEF_USER].tri = no;
+ }
+ csym->flags |= SYMBOL_DEF_USER;
+ /* clear VALID to get value calculated */
+ csym->flags &= ~(SYMBOL_VALID);
+}
+
+void conf_set_all_new_symbols(enum conf_def_mode mode)
+{
+ struct symbol *sym, *csym;
+ int i, cnt;
for_all_symbols(i, sym) {
if (sym_has_value(sym))
@@ -961,8 +1021,6 @@ void conf_set_all_new_symbols(enum conf_def_mode mode)
sym_clear_all_valid();
- if (mode != def_random)
- return;
/*
* We have different type of choice blocks.
* If curr.tri equal to mod then we can select several
@@ -977,35 +1035,9 @@ void conf_set_all_new_symbols(enum conf_def_mode mode)
continue;
sym_calc_value(csym);
-
- if (csym->curr.tri != yes)
- continue;
-
- prop = sym_get_choice_prop(csym);
-
- /* count entries in choice block */
- cnt = 0;
- expr_list_for_each_sym(prop->expr, e, sym)
- cnt++;
-
- /*
- * find a random value and set it to yes,
- * set the rest to no so we have only one set
- */
- def = (rand() % cnt);
-
- cnt = 0;
- expr_list_for_each_sym(prop->expr, e, sym) {
- if (def == cnt++) {
- sym->def[S_DEF_USER].tri = yes;
- csym->def[S_DEF_USER].val = sym;
- }
- else {
- sym->def[S_DEF_USER].tri = no;
- }
- }
- csym->flags |= SYMBOL_DEF_USER;
- /* clear VALID to get value calculated */
- csym->flags &= ~(SYMBOL_VALID);
+ if (mode == def_random)
+ randomize_choice_values(csym);
+ else
+ set_all_choice_values(csym);
}
}
--
1.6.0.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists