linux-kernel - Re: [PATCH] kconfig: make 'imply' obey the direct dependency

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]

Message-ID: <CAK7LNASubs2oJ8-a-amsEhWjYrfpAx8QxeTzH4PnKsELKyLaUw@mail.gmail.com>
Date:   Thu, 20 Feb 2020 16:45:13 +0900
From:   Masahiro Yamada <masahiroy@...nel.org>
To:     Nicolas Pitre <nico@...xnic.net>
Cc:     Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Richard Cochran <richardcochran@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        John Stultz <john.stultz@...aro.org>,
        Josh Triplett <josh@...htriplett.org>,
        Mark Brown <broonie@...nel.org>,
        Ulf Magnusson <ulfalizer@...il.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Jonathan Corbet <corbet@....net>,
        Michal Marek <michal.lkml@...kovi.net>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] kconfig: make 'imply' obey the direct dependency

Hi Nicolas,

On Thu, Feb 20, 2020 at 1:16 AM Nicolas Pitre <nico@...xnic.net> wrote:
>
> On Wed, 19 Feb 2020, Masahiro Yamada wrote:
>
> > The 'imply' statement may create unmet direct dependency when the
> > implied symbol depends on m.
> >
> > [Test Code]
> >
> >   config FOO
> >           tristate "foo"
> >           imply BAZ
> >
> >   config BAZ
> >           tristate "baz"
> >           depends on BAR
> >
> >   config BAR
> >           def_tristate m
> >
> >   config MODULES
> >           def_bool y
> >           option modules
> >
> > If you set FOO=y, BAZ is also promoted to y, which results in the
> > following .config file:
> >
> >   CONFIG_FOO=y
> >   CONFIG_BAZ=y
> >   CONFIG_BAR=m
> >   CONFIG_MODULES=y
> >
> > This ignores the dependency "BAZ depends on BAR".
> >
> > Unlike 'select', what is worse, Kconfig never shows the
> > "WARNING: unmet direct dependencies detected for ..." for this case.
> >
> > Because 'imply' should be weaker than 'depends on', Kconfig should
> > take the direct dependency into account.
> >
> > Describe this case in Documentation/kbuild/kconfig-language.rst for
> > clarification.
> >
> > Commit 237e3ad0f195 ("Kconfig: Introduce the "imply" keyword") says that
> > a symbol implied by y is restricted to y or n, excluding m.
> >
> > As for the combination of FOO=y and BAR=m, the case of BAZ=m is excluded
> > by the 'imply', and BAZ=y is also excluded by 'depends on'. So, only the
> > possible value is BAZ=n.
>
> I don't think this is right. The imply keyword provide influence over
> another symbol but it should not impose any restrictions. If BAR=m then
> BAZ should still be allowed to be m or n.
>
> > @@ -174,6 +174,9 @@ applicable everywhere (see syntax).
> >       n               y               n               N/m/y
> >       m               y               m               M/y/n
> >       y               y               y               Y/n
> > +     n               m               n               N/m
> > +     m               m               m               M/n
> > +     y               m               n               N
>
> Here the last line shoule be y m n N/m.
>
> Generally speaking, the code enabled by FOO may rely on functionalities
> provided by BAZ only when BAZ >= FOO. This is accomplished with
> IS_REACHABLE():
>
>         foo_init()
>         {
>                 if (IS_REACHABLE(CONFIG_BAZ))
>                         baz_register(&foo);
>                 ...
>         }
>
> So if FOO=y and BAZ=m then IS_REACHABLE(CONFIG_BAZ) will be false. Maybe
> adding a note to that effect linked to the "y m n N/m" line in the table
> would be a good idea.
>

I also thought so.

I agree IS_REACHABLE() is much saner approach.

So, do you agree to change the current behavior
as follows?


index d0111dd26410..47dbfd1ee003 100644
--- a/Documentation/kbuild/kconfig-language.rst
+++ b/Documentation/kbuild/kconfig-language.rst
@@ -173,7 +173,7 @@ applicable everywhere (see syntax).
        ===             ===             =============   ==============
        n               y               n               N/m/y
        m               y               m               M/y/n
-       y               y               y               Y/n
+       y               y               y               Y/m/n
        y               n               *               N
        ===             ===             =============   ==============






-- 
Best Regards
Masahiro Yamada