2016-04-02 17:55:40

by Benjamin Poirier

[permalink] [raw]
Subject: [PATCH 1/2] localmodconfig: Fix parsing of Kconfig "source" statements

The parameter of Kconfig "source" statements does not need to be quoted.
The current regex causes many kconfig files to be skipped and hence,
dependencies to be missed.

Also fix the whitespace repeat count.

Signed-off-by: Benjamin Poirier <[email protected]>
---
scripts/kconfig/streamline_config.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index f3d3fb4..7036ae3 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -188,7 +188,7 @@ sub read_kconfig {
$cont = 0;

# collect any Kconfig sources
- if (/^source\s*"(.*)"/) {
+ if (/^source\s+"?([^"]+)/) {
my $kconfig = $1;
# prevent reading twice.
if (!defined($read_kconfigs{$kconfig})) {
--
2.7.2


2016-04-02 17:55:49

by Benjamin Poirier

[permalink] [raw]
Subject: [PATCH 2/2] localmodconfig: Reset certificate paths

When using `make localmodconfig` and friends, if the input config comes
from a kernel that was built in a different environment (for example, the
canonical case of using localmodconfig to trim a distribution kernel
config) the key files for module signature checking will not be available
and should be regenerated or omitted. Otherwise, the user will be faced
with annoying errors when trying to build with the generated .config:

make[1]: *** No rule to make target 'keyring.crt', needed by 'certs/x509_certificate_list'. Stop.
Makefile:1576: recipe for target 'certs/' failed

Signed-off-by: Benjamin Poirier <[email protected]>
---
scripts/kconfig/streamline_config.pl | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index 7036ae3..514735d 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -610,6 +610,40 @@ foreach my $line (@config_file) {
next;
}

+ if (/CONFIG_MODULE_SIG_KEY="(.+)"/) {
+ my $orig_cert = $1;
+ my $default_cert = "certs/signing_key.pem";
+
+ # Check that the logic in this script still matches the one in Kconfig
+ if (!defined($depends{"MODULE_SIG_KEY"}) ||
+ $depends{"MODULE_SIG_KEY"} !~ /"\Q$default_cert\E"/) {
+ die "Assertion failure, update needed";
+ }
+
+ if ($orig_cert ne $default_cert && ! -f $orig_cert) {
+ print STDERR "Module signature verification enabled but ",
+ "module signing key \"$orig_cert\" not found. Resetting ",
+ "signing key to default value.\n";
+ print "CONFIG_MODULE_SIG_KEY=\"$default_cert\"\n";
+ } else {
+ print;
+ }
+ next;
+ }
+
+ if (/CONFIG_SYSTEM_TRUSTED_KEYS="(.+)"/) {
+ my $orig_keys = $1;
+
+ if (! -f $orig_keys) {
+ print STDERR "System keyring enabled but keys \"$orig_keys\" ",
+ "not found. Resetting keys to default value.\n";
+ print "CONFIG_SYSTEM_TRUSTED_KEYS=\"\"\n";
+ } else {
+ print;
+ }
+ next;
+ }
+
if (/^(CONFIG.*)=(m|y)/) {
if (defined($configs{$1})) {
if ($localyesconfig) {
--
2.7.2

2016-04-08 14:59:17

by joeyli

[permalink] [raw]
Subject: Re: [PATCH 1/2] localmodconfig: Fix parsing of Kconfig "source" statements

On Sat, Apr 02, 2016 at 10:55:21AM -0700, Benjamin Poirier wrote:
> The parameter of Kconfig "source" statements does not need to be quoted.
> The current regex causes many kconfig files to be skipped and hence,
> dependencies to be missed.
>
> Also fix the whitespace repeat count.
>
> Signed-off-by: Benjamin Poirier <[email protected]>

Tested-by: Lee, Chun-Yi <[email protected]>

> ---
> scripts/kconfig/streamline_config.pl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index f3d3fb4..7036ae3 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -188,7 +188,7 @@ sub read_kconfig {
> $cont = 0;
>
> # collect any Kconfig sources
> - if (/^source\s*"(.*)"/) {
> + if (/^source\s+"?([^"]+)/) {
> my $kconfig = $1;
> # prevent reading twice.
> if (!defined($read_kconfigs{$kconfig})) {
> --
> 2.7.2

Regards
Joey Lee

2016-04-08 15:00:10

by joeyli

[permalink] [raw]
Subject: Re: [PATCH 2/2] localmodconfig: Reset certificate paths

On Sat, Apr 02, 2016 at 10:55:22AM -0700, Benjamin Poirier wrote:
> When using `make localmodconfig` and friends, if the input config comes
> from a kernel that was built in a different environment (for example, the
> canonical case of using localmodconfig to trim a distribution kernel
> config) the key files for module signature checking will not be available
> and should be regenerated or omitted. Otherwise, the user will be faced
> with annoying errors when trying to build with the generated .config:
>
> make[1]: *** No rule to make target 'keyring.crt', needed by 'certs/x509_certificate_list'. Stop.
> Makefile:1576: recipe for target 'certs/' failed
>
> Signed-off-by: Benjamin Poirier <[email protected]>

Tested-by: Lee, Chun-Yi <[email protected]>


Regards
Joey Lee

> ---
> scripts/kconfig/streamline_config.pl | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index 7036ae3..514735d 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -610,6 +610,40 @@ foreach my $line (@config_file) {
> next;
> }
>
> + if (/CONFIG_MODULE_SIG_KEY="(.+)"/) {
> + my $orig_cert = $1;
> + my $default_cert = "certs/signing_key.pem";
> +
> + # Check that the logic in this script still matches the one in Kconfig
> + if (!defined($depends{"MODULE_SIG_KEY"}) ||
> + $depends{"MODULE_SIG_KEY"} !~ /"\Q$default_cert\E"/) {
> + die "Assertion failure, update needed";
> + }
> +
> + if ($orig_cert ne $default_cert && ! -f $orig_cert) {
> + print STDERR "Module signature verification enabled but ",
> + "module signing key \"$orig_cert\" not found. Resetting ",
> + "signing key to default value.\n";
> + print "CONFIG_MODULE_SIG_KEY=\"$default_cert\"\n";
> + } else {
> + print;
> + }
> + next;
> + }
> +
> + if (/CONFIG_SYSTEM_TRUSTED_KEYS="(.+)"/) {
> + my $orig_keys = $1;
> +
> + if (! -f $orig_keys) {
> + print STDERR "System keyring enabled but keys \"$orig_keys\" ",
> + "not found. Resetting keys to default value.\n";
> + print "CONFIG_SYSTEM_TRUSTED_KEYS=\"\"\n";
> + } else {
> + print;
> + }
> + next;
> + }
> +
> if (/^(CONFIG.*)=(m|y)/) {
> if (defined($configs{$1})) {
> if ($localyesconfig) {
> --
> 2.7.2

2016-04-08 18:29:45

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 1/2] localmodconfig: Fix parsing of Kconfig "source" statements

On Sat, 2 Apr 2016 10:55:21 -0700
Benjamin Poirier <[email protected]> wrote:

> The parameter of Kconfig "source" statements does not need to be quoted.
> The current regex causes many kconfig files to be skipped and hence,
> dependencies to be missed.
>
> Also fix the whitespace repeat count.
>
> Signed-off-by: Benjamin Poirier <[email protected]>

Thanks for sending this. I'll apply it. Should this be marked for
stable? And if so, how far back?

-- Steve

> ---
> scripts/kconfig/streamline_config.pl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index f3d3fb4..7036ae3 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -188,7 +188,7 @@ sub read_kconfig {
> $cont = 0;
>
> # collect any Kconfig sources
> - if (/^source\s*"(.*)"/) {
> + if (/^source\s+"?([^"]+)/) {
> my $kconfig = $1;
> # prevent reading twice.
> if (!defined($read_kconfigs{$kconfig})) {

2016-04-10 23:52:45

by Benjamin Poirier

[permalink] [raw]
Subject: Re: [PATCH 1/2] localmodconfig: Fix parsing of Kconfig "source" statements

On 2016/04/08 14:29, Steven Rostedt wrote:
> On Sat, 2 Apr 2016 10:55:21 -0700
> Benjamin Poirier <[email protected]> wrote:
>
> > The parameter of Kconfig "source" statements does not need to be quoted.
> > The current regex causes many kconfig files to be skipped and hence,
> > dependencies to be missed.
> >
> > Also fix the whitespace repeat count.
> >
> > Signed-off-by: Benjamin Poirier <[email protected]>
>
> Thanks for sending this. I'll apply it. Should this be marked for
> stable? And if so, how far back?

The first problem dates back to the introduction of streamline_config.pl in
dcc6024 kconfig: add streamline_config.pl to scripts (v2.6.32-rc1)
The second problem started with
19e91b6 modsign: Allow external signing key to be specified (v4.3-rc1)

However, I'm not sure that adding the patch to stable is warranted. I
considered this with regards to the problem fixed by patch 1/2.

First, I searched for cases where dependency info that's currently missing
would lead to lead to symbols being erroneously deactivated but I did not find
any. Since these cases are not so obvious and are quite rare in general, it's
possible that I missed one but at this stage the problem is theoretical.

Second, even if there is such a case, I'm not sure that the problem is
"critical". streamline_config.pl may output an "invalid" config because
it misses some dependencies but the config will be fixed by the
invocation of silentoldconfig that comes right after in the
localmodconfig makefile rule. It might not be the config the user
wanted, but it will be valid.

However, while looking for such a case, I've noticed a few other issues in
streamline_config.pl and I'll send more patches shortly.

>
> -- Steve
>
> > ---
> > scripts/kconfig/streamline_config.pl | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> > index f3d3fb4..7036ae3 100755
> > --- a/scripts/kconfig/streamline_config.pl
> > +++ b/scripts/kconfig/streamline_config.pl
> > @@ -188,7 +188,7 @@ sub read_kconfig {
> > $cont = 0;
> >
> > # collect any Kconfig sources
> > - if (/^source\s*"(.*)"/) {
> > + if (/^source\s+"?([^"]+)/) {
> > my $kconfig = $1;
> > # prevent reading twice.
> > if (!defined($read_kconfigs{$kconfig})) {
>
>

2016-04-11 00:06:55

by Benjamin Poirier

[permalink] [raw]
Subject: [PATCH 1/4] localmodconfig: Recognize more keywords that end a menu entry

Based on the list in Documentation/kbuild/kconfig-language.txt

This removes junk from %depends because parsing of a menu entry spilled
over to another menu entry.

Signed-off-by: Benjamin Poirier <[email protected]>
---
scripts/kconfig/streamline_config.pl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index 514735d..c5b388c 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -256,8 +256,8 @@ sub read_kconfig {

$iflevel-- if ($iflevel);

- # stop on "help"
- } elsif (/^\s*help\s*$/) {
+ # stop on "help" and keywords that end a menu entry
+ } elsif (/^\s*help\s*$/ || /^(comment|choice|menu)\b/) {
$state = "NONE";
}
}
--
2.7.4

2016-04-11 00:07:03

by Benjamin Poirier

[permalink] [raw]
Subject: [PATCH 2/4] localmodconfig: Fix parsing of "help" text

Help text may start with "help" or "---help---". This patch fixes
read_kconfig() to recognize the second variant.

This removes useless junk from %depends and %selects. That junk is due to
help text that contains the words "selects" and "depends".

Signed-off-by: Benjamin Poirier <[email protected]>
---
scripts/kconfig/streamline_config.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index c5b388c..f06972a 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -257,7 +257,7 @@ sub read_kconfig {
$iflevel-- if ($iflevel);

# stop on "help" and keywords that end a menu entry
- } elsif (/^\s*help\s*$/ || /^(comment|choice|menu)\b/) {
+ } elsif (/^\s*(---)?help(---)?\s*$/ || /^(comment|choice|menu)\b/) {
$state = "NONE";
}
}
--
2.7.4

2016-04-11 00:07:12

by Benjamin Poirier

[permalink] [raw]
Subject: [PATCH 3/4] localmodconfig: Add missing $ to reference a variable

That is clearly what the original intention was. This does not change the
output .config but it prevents some useless processing.

! eq "m" is changed to the simpler eq "y"; symbols with values other than
m|y are not included in %orig_configs.

Signed-off-by: Benjamin Poirier <[email protected]>
---
scripts/kconfig/streamline_config.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index f06972a..bbc160c 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -454,7 +454,7 @@ sub parse_config_depends
$p =~ s/^[^$valid]*[$valid]+//;

# We only need to process if the depend config is a module
- if (!defined($orig_configs{$conf}) || !$orig_configs{conf} eq "m") {
+ if (!defined($orig_configs{$conf}) || $orig_configs{$conf} eq "y") {
next;
}

--
2.7.4

2016-04-11 00:07:34

by Benjamin Poirier

[permalink] [raw]
Subject: [PATCH 4/4] localmodconfig: Recognize standalone "prompt"

Note that this may change the resulting .config, causing it to have fewer
symbols turned on. Before this patch we incorrectly identified some symbols
as not having a prompt and needing to be selected by something else.

Also fix the whitespace repeat after "tristate".

Signed-off-by: Benjamin Poirier <[email protected]>
---
scripts/kconfig/streamline_config.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index bbc160c..d465672 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -237,7 +237,7 @@ sub read_kconfig {
}

# configs without prompts must be selected
- } elsif ($state ne "NONE" && /^\s*tristate\s\S/) {
+ } elsif ($state ne "NONE" && /^\s*(tristate\s+\S|prompt\b)/) {
# note if the config has a prompt
$prompts{$config} = 1;

--
2.7.4

2016-04-18 20:29:22

by Benjamin Poirier

[permalink] [raw]
Subject: Re: [PATCH 1/2] localmodconfig: Fix parsing of Kconfig "source" statements

On 2016/04/10 16:52, Benjamin Poirier wrote:
> On 2016/04/08 14:29, Steven Rostedt wrote:
> > On Sat, 2 Apr 2016 10:55:21 -0700
> > Benjamin Poirier <[email protected]> wrote:
> >
> > > The parameter of Kconfig "source" statements does not need to be quoted.
> > > The current regex causes many kconfig files to be skipped and hence,
> > > dependencies to be missed.
> > >
> > > Also fix the whitespace repeat count.
> > >
> > > Signed-off-by: Benjamin Poirier <[email protected]>
> >
> > Thanks for sending this. I'll apply it. Should this be marked for
> > stable? And if so, how far back?
>
> The first problem dates back to the introduction of streamline_config.pl in
> dcc6024 kconfig: add streamline_config.pl to scripts (v2.6.32-rc1)
> The second problem started with
> 19e91b6 modsign: Allow external signing key to be specified (v4.3-rc1)
>
> However, I'm not sure that adding the patch to stable is warranted. I
> considered this with regards to the problem fixed by patch 1/2.
>
> First, I searched for cases where dependency info that's currently missing
> would lead to lead to symbols being erroneously deactivated but I did not find
> any. Since these cases are not so obvious and are quite rare in general, it's
> possible that I missed one but at this stage the problem is theoretical.
>
> Second, even if there is such a case, I'm not sure that the problem is
> "critical". streamline_config.pl may output an "invalid" config because
> it misses some dependencies but the config will be fixed by the
> invocation of silentoldconfig that comes right after in the
> localmodconfig makefile rule. It might not be the config the user
> wanted, but it will be valid.
>
> However, while looking for such a case, I've noticed a few other issues in
> streamline_config.pl and I'll send more patches shortly.

Hi Steven,

Can you please merge these six patches (2 original + 4 followups)?

Thank you,
-Benjamin

>
> >
> > -- Steve
> >
> > > ---
> > > scripts/kconfig/streamline_config.pl | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> > > index f3d3fb4..7036ae3 100755
> > > --- a/scripts/kconfig/streamline_config.pl
> > > +++ b/scripts/kconfig/streamline_config.pl
> > > @@ -188,7 +188,7 @@ sub read_kconfig {
> > > $cont = 0;
> > >
> > > # collect any Kconfig sources
> > > - if (/^source\s*"(.*)"/) {
> > > + if (/^source\s+"?([^"]+)/) {
> > > my $kconfig = $1;
> > > # prevent reading twice.
> > > if (!defined($read_kconfigs{$kconfig})) {
> >
> >

2016-04-18 21:43:37

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 1/2] localmodconfig: Fix parsing of Kconfig "source" statements

On Mon, 18 Apr 2016 13:29:12 -0700
Benjamin Poirier <[email protected]> wrote:

> On 2016/04/10 16:52, Benjamin Poirier wrote:
> > On 2016/04/08 14:29, Steven Rostedt wrote:
> > > On Sat, 2 Apr 2016 10:55:21 -0700
> > > Benjamin Poirier <[email protected]> wrote:
> > >
> > > > The parameter of Kconfig "source" statements does not need to be quoted.
> > > > The current regex causes many kconfig files to be skipped and hence,
> > > > dependencies to be missed.
> > > >
> > > > Also fix the whitespace repeat count.
> > > >
> > > > Signed-off-by: Benjamin Poirier <[email protected]>
> > >
> > > Thanks for sending this. I'll apply it. Should this be marked for
> > > stable? And if so, how far back?
> >
> > The first problem dates back to the introduction of streamline_config.pl in
> > dcc6024 kconfig: add streamline_config.pl to scripts (v2.6.32-rc1)
> > The second problem started with
> > 19e91b6 modsign: Allow external signing key to be specified (v4.3-rc1)
> >
> > However, I'm not sure that adding the patch to stable is warranted. I
> > considered this with regards to the problem fixed by patch 1/2.
> >
> > First, I searched for cases where dependency info that's currently missing
> > would lead to lead to symbols being erroneously deactivated but I did not find
> > any. Since these cases are not so obvious and are quite rare in general, it's
> > possible that I missed one but at this stage the problem is theoretical.
> >
> > Second, even if there is such a case, I'm not sure that the problem is
> > "critical". streamline_config.pl may output an "invalid" config because
> > it misses some dependencies but the config will be fixed by the
> > invocation of silentoldconfig that comes right after in the
> > localmodconfig makefile rule. It might not be the config the user
> > wanted, but it will be valid.
> >
> > However, while looking for such a case, I've noticed a few other issues in
> > streamline_config.pl and I'll send more patches shortly.
>
> Hi Steven,
>
> Can you please merge these six patches (2 original + 4 followups)?
>

OK, I've marked them as todo. I've just come back from two weeks of
conferences and I'm working on catching up.

-- Steve

2016-04-18 23:50:07

by Benjamin Poirier

[permalink] [raw]
Subject: Re: [PATCH 1/2] localmodconfig: Fix parsing of Kconfig "source" statements

On 2016/04/18 17:43, Steven Rostedt wrote:
[...]
> > Hi Steven,
> >
> > Can you please merge these six patches (2 original + 4 followups)?
> >
>
> OK, I've marked them as todo. I've just come back from two weeks of
> conferences and I'm working on catching up.
>

Ah I see, thanks. I just wanted to be sure they didn't fall through the
cracks.

2016-04-26 14:02:20

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 2/2] localmodconfig: Reset certificate paths

On Sat, 2 Apr 2016 10:55:22 -0700
Benjamin Poirier <[email protected]> wrote:

> When using `make localmodconfig` and friends, if the input config comes
> from a kernel that was built in a different environment (for example, the
> canonical case of using localmodconfig to trim a distribution kernel
> config) the key files for module signature checking will not be available
> and should be regenerated or omitted. Otherwise, the user will be faced
> with annoying errors when trying to build with the generated .config:
>
> make[1]: *** No rule to make target 'keyring.crt', needed by 'certs/x509_certificate_list'. Stop.
> Makefile:1576: recipe for target 'certs/' failed
>
> Signed-off-by: Benjamin Poirier <[email protected]>
> ---
> scripts/kconfig/streamline_config.pl | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index 7036ae3..514735d 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -610,6 +610,40 @@ foreach my $line (@config_file) {
> next;
> }
>
> + if (/CONFIG_MODULE_SIG_KEY="(.+)"/) {
> + my $orig_cert = $1;
> + my $default_cert = "certs/signing_key.pem";
> +
> + # Check that the logic in this script still matches the one in Kconfig
> + if (!defined($depends{"MODULE_SIG_KEY"}) ||
> + $depends{"MODULE_SIG_KEY"} !~ /"\Q$default_cert\E"/) {
> + die "Assertion failure, update needed";

Instead of dieing here, what about just going back to the current
behavior, and ignore the sig keys?

-- Steve

> + }
> +
> + if ($orig_cert ne $default_cert && ! -f $orig_cert) {
> + print STDERR "Module signature verification enabled but ",
> + "module signing key \"$orig_cert\" not found. Resetting ",
> + "signing key to default value.\n";
> + print "CONFIG_MODULE_SIG_KEY=\"$default_cert\"\n";
> + } else {
> + print;
> + }
> + next;
> + }
> +
> + if (/CONFIG_SYSTEM_TRUSTED_KEYS="(.+)"/) {
> + my $orig_keys = $1;
> +
> + if (! -f $orig_keys) {
> + print STDERR "System keyring enabled but keys \"$orig_keys\" ",
> + "not found. Resetting keys to default value.\n";
> + print "CONFIG_SYSTEM_TRUSTED_KEYS=\"\"\n";
> + } else {
> + print;
> + }
> + next;
> + }
> +
> if (/^(CONFIG.*)=(m|y)/) {
> if (defined($configs{$1})) {
> if ($localyesconfig) {

2016-04-26 14:03:36

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 1/2] localmodconfig: Fix parsing of Kconfig "source" statements

On Fri, 8 Apr 2016 22:59:02 +0800
joeyli <[email protected]> wrote:

> On Sat, Apr 02, 2016 at 10:55:21AM -0700, Benjamin Poirier wrote:
> > The parameter of Kconfig "source" statements does not need to be quoted.
> > The current regex causes many kconfig files to be skipped and hence,
> > dependencies to be missed.
> >
> > Also fix the whitespace repeat count.
> >
> > Signed-off-by: Benjamin Poirier <[email protected]>
>
> Tested-by: Lee, Chun-Yi <[email protected]>
>

Applied.

Thanks!

-- Steve

2016-04-26 15:11:42

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 4/4] localmodconfig: Recognize standalone "prompt"

On Sun, 10 Apr 2016 17:06:33 -0700
Benjamin Poirier <[email protected]> wrote:

> Note that this may change the resulting .config, causing it to have fewer
> symbols turned on. Before this patch we incorrectly identified some symbols
> as not having a prompt and needing to be selected by something else.
>
> Also fix the whitespace repeat after "tristate".
>
> Signed-off-by: Benjamin Poirier <[email protected]>
> ---
> scripts/kconfig/streamline_config.pl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index bbc160c..d465672 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -237,7 +237,7 @@ sub read_kconfig {
> }
>
> # configs without prompts must be selected
> - } elsif ($state ne "NONE" && /^\s*tristate\s\S/) {
> + } elsif ($state ne "NONE" && /^\s*(tristate\s+\S|prompt\b)/) {

I'm curious. What modules have tristate and a specified prompt?

That is, what modules are removed with this patch?

I applied the first three so far.

Thanks,

-- Steve

> # note if the config has a prompt
> $prompts{$config} = 1;
>

2016-04-26 18:51:31

by Benjamin Poirier

[permalink] [raw]
Subject: Re: [PATCH 2/2] localmodconfig: Reset certificate paths

On 2016/04/26 10:02, Steven Rostedt wrote:
> On Sat, 2 Apr 2016 10:55:22 -0700
> Benjamin Poirier <[email protected]> wrote:
>
> > When using `make localmodconfig` and friends, if the input config comes
> > from a kernel that was built in a different environment (for example, the
> > canonical case of using localmodconfig to trim a distribution kernel
> > config) the key files for module signature checking will not be available
> > and should be regenerated or omitted. Otherwise, the user will be faced
> > with annoying errors when trying to build with the generated .config:
> >
> > make[1]: *** No rule to make target 'keyring.crt', needed by 'certs/x509_certificate_list'. Stop.
> > Makefile:1576: recipe for target 'certs/' failed
> >
> > Signed-off-by: Benjamin Poirier <[email protected]>
> > ---
> > scripts/kconfig/streamline_config.pl | 34 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 34 insertions(+)
> >
> > diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> > index 7036ae3..514735d 100755
> > --- a/scripts/kconfig/streamline_config.pl
> > +++ b/scripts/kconfig/streamline_config.pl
> > @@ -610,6 +610,40 @@ foreach my $line (@config_file) {
> > next;
> > }
> >
> > + if (/CONFIG_MODULE_SIG_KEY="(.+)"/) {
> > + my $orig_cert = $1;
> > + my $default_cert = "certs/signing_key.pem";
> > +
> > + # Check that the logic in this script still matches the one in Kconfig
> > + if (!defined($depends{"MODULE_SIG_KEY"}) ||
> > + $depends{"MODULE_SIG_KEY"} !~ /"\Q$default_cert\E"/) {
> > + die "Assertion failure, update needed";
>
> Instead of dieing here, what about just going back to the current
> behavior, and ignore the sig keys?

I was concerned that the warning may go unnoticed but I think you're right. It
is the same kind of concern between a BUG() or a WARN_ON(). In this case it
certainly is possible to keep going and ignore the certificate check, as you
suggest.

2016-04-26 18:52:22

by Benjamin Poirier

[permalink] [raw]
Subject: [PATCH v2] localmodconfig: Reset certificate paths

When using `make localmodconfig` and friends, if the input config comes
from a kernel that was built in a different environment (for example, the
canonical case of using localmodconfig to trim a distribution kernel
config) the key files for module signature checking will not be available
and should be regenerated or omitted. Otherwise, the user will be faced
with annoying errors when trying to build with the generated .config:

make[1]: *** No rule to make target 'keyring.crt', needed by 'certs/x509_certificate_list'. Stop.
Makefile:1576: recipe for target 'certs/' failed

Signed-off-by: Benjamin Poirier <[email protected]>
---
scripts/kconfig/streamline_config.pl | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index 01b53ec..95a6f2b 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -610,6 +610,40 @@ foreach my $line (@config_file) {
next;
}

+ if (/CONFIG_MODULE_SIG_KEY="(.+)"/) {
+ my $orig_cert = $1;
+ my $default_cert = "certs/signing_key.pem";
+
+ # Check that the logic in this script still matches the one in Kconfig
+ if (!defined($depends{"MODULE_SIG_KEY"}) ||
+ $depends{"MODULE_SIG_KEY"} !~ /"\Q$default_cert\E"/) {
+ print STDERR "WARNING: MODULE_SIG_KEY assertion failure, ",
+ "update needed to ", __FILE__, " line ", __LINE__, "\n";
+ print;
+ } elsif ($orig_cert ne $default_cert && ! -f $orig_cert) {
+ print STDERR "Module signature verification enabled but ",
+ "module signing key \"$orig_cert\" not found. Resetting ",
+ "signing key to default value.\n";
+ print "CONFIG_MODULE_SIG_KEY=\"$default_cert\"\n";
+ } else {
+ print;
+ }
+ next;
+ }
+
+ if (/CONFIG_SYSTEM_TRUSTED_KEYS="(.+)"/) {
+ my $orig_keys = $1;
+
+ if (! -f $orig_keys) {
+ print STDERR "System keyring enabled but keys \"$orig_keys\" ",
+ "not found. Resetting keys to default value.\n";
+ print "CONFIG_SYSTEM_TRUSTED_KEYS=\"\"\n";
+ } else {
+ print;
+ }
+ next;
+ }
+
if (/^(CONFIG.*)=(m|y)/) {
if (defined($configs{$1})) {
if ($localyesconfig) {
--
2.7.4

2016-04-26 18:54:30

by Benjamin Poirier

[permalink] [raw]
Subject: Re: [PATCH 4/4] localmodconfig: Recognize standalone "prompt"

On 2016/04/26 11:11, Steven Rostedt wrote:
> On Sun, 10 Apr 2016 17:06:33 -0700
> Benjamin Poirier <[email protected]> wrote:
>
> > Note that this may change the resulting .config, causing it to have fewer
> > symbols turned on. Before this patch we incorrectly identified some symbols
> > as not having a prompt and needing to be selected by something else.
> >
> > Also fix the whitespace repeat after "tristate".
> >
> > Signed-off-by: Benjamin Poirier <[email protected]>
> > ---
> > scripts/kconfig/streamline_config.pl | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> > index bbc160c..d465672 100755
> > --- a/scripts/kconfig/streamline_config.pl
> > +++ b/scripts/kconfig/streamline_config.pl
> > @@ -237,7 +237,7 @@ sub read_kconfig {
> > }
> >
> > # configs without prompts must be selected
> > - } elsif ($state ne "NONE" && /^\s*tristate\s\S/) {
> > + } elsif ($state ne "NONE" && /^\s*(tristate\s+\S|prompt\b)/) {
>
> I'm curious. What modules have tristate and a specified prompt?

Actually, I didn't find config symbols that are tristate and have a separate
prompt. However, the following tristate symbols were not recognized by the
original expression because they are defined with two spaces between
"tristate" and the prompt string (sigh):
IP6_NF_RAW
IP_NF_RAW
NETFILTER_XT_MATCH_COMMENT
NETFILTER_XT_MATCH_CONNBYTES
NETFILTER_XT_MATCH_CONNMARK
NETFILTER_XT_MATCH_REALM
NETFILTER_XT_MATCH_SCTP
NETFILTER_XT_MATCH_STRING
NETFILTER_XT_TARGET_CONNMARK
NETFILTER_XT_TARGET_IDLETIMER
NETFILTER_XT_TARGET_NOTRACK
NETFILTER_XT_TARGET_TRACE
NF_CT_NETLINK_TIMEOUT
USB_EHCI_HCD_AT91
USB_EHCI_HCD_ORION

So they were not added to %prompts and now they are.

>
> That is, what modules are removed with this patch?
>

None, it turns out. Because it is the case that none of the above symbols are
selected by other symbols; the second condition in
if (!defined($prompts{$config}) && defined($selects{$config})) {
$process_selects{$config} = 1;
}
is always false for these symbols.

I've updated the patch subject and log accordingly.

2016-04-26 18:56:50

by Benjamin Poirier

[permalink] [raw]
Subject: [PATCH v2] localmodconfig: Fix whitespace repeat count after "tristate"

Also recognize standalone "prompt".

Before this patch we incorrectly identified some symbols as not having a
prompt and potentially needing to be selected by something else.

Note that this patch could theoretically change the resulting .config,
causing it to have fewer symbols turned on. However, given the current set
of Kconfig files, this situation does not occur because the symbols newly
added to %prompts are absent from %selects.

Signed-off-by: Benjamin Poirier <[email protected]>
---
scripts/kconfig/streamline_config.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index 95a6f2b..b8c7b29 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -237,7 +237,7 @@ sub read_kconfig {
}

# configs without prompts must be selected
- } elsif ($state ne "NONE" && /^\s*tristate\s\S/) {
+ } elsif ($state ne "NONE" && /^\s*(tristate\s+\S|prompt\b)/) {
# note if the config has a prompt
$prompts{$config} = 1;

--
2.7.4

2016-04-26 19:48:47

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v2] localmodconfig: Reset certificate paths

On Tue, 26 Apr 2016 11:52:01 -0700
Benjamin Poirier <[email protected]> wrote:

> When using `make localmodconfig` and friends, if the input config comes
> from a kernel that was built in a different environment (for example, the
> canonical case of using localmodconfig to trim a distribution kernel
> config) the key files for module signature checking will not be available
> and should be regenerated or omitted. Otherwise, the user will be faced
> with annoying errors when trying to build with the generated .config:
>
> make[1]: *** No rule to make target 'keyring.crt', needed by 'certs/x509_certificate_list'. Stop.
> Makefile:1576: recipe for target 'certs/' failed
>
> Signed-off-by: Benjamin Poirier <[email protected]>

Applied, thanks!

-- Steve

2016-04-26 19:51:38

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v2] localmodconfig: Fix whitespace repeat count after "tristate"

On Tue, 26 Apr 2016 11:56:38 -0700
Benjamin Poirier <[email protected]> wrote:

> Also recognize standalone "prompt".
>
> Before this patch we incorrectly identified some symbols as not having a
> prompt and potentially needing to be selected by something else.
>
> Note that this patch could theoretically change the resulting .config,
> causing it to have fewer symbols turned on. However, given the current set
> of Kconfig files, this situation does not occur because the symbols newly
> added to %prompts are absent from %selects.
>
> Signed-off-by: Benjamin Poirier <[email protected]>
> ---
> scripts/kconfig/streamline_config.pl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index 95a6f2b..b8c7b29 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -237,7 +237,7 @@ sub read_kconfig {
> }
>
> # configs without prompts must be selected
> - } elsif ($state ne "NONE" && /^\s*tristate\s\S/) {
> + } elsif ($state ne "NONE" && /^\s*(tristate\s+\S|prompt\b)/) {

I prefer not to have the "prompt" here. I'm only interested in module
configs. But the \s to \s+ is a real fix.

Can you resend?

-- Steve

> # note if the config has a prompt
> $prompts{$config} = 1;
>

2016-04-26 21:35:27

by Benjamin Poirier

[permalink] [raw]
Subject: Re: [PATCH v2] localmodconfig: Fix whitespace repeat count after "tristate"

On 2016/04/26 15:51, Steven Rostedt wrote:
> On Tue, 26 Apr 2016 11:56:38 -0700
> Benjamin Poirier <[email protected]> wrote:
>
> > Also recognize standalone "prompt".
> >
> > Before this patch we incorrectly identified some symbols as not having a
> > prompt and potentially needing to be selected by something else.
> >
> > Note that this patch could theoretically change the resulting .config,
> > causing it to have fewer symbols turned on. However, given the current set
> > of Kconfig files, this situation does not occur because the symbols newly
> > added to %prompts are absent from %selects.
> >
> > Signed-off-by: Benjamin Poirier <[email protected]>
> > ---
> > scripts/kconfig/streamline_config.pl | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> > index 95a6f2b..b8c7b29 100755
> > --- a/scripts/kconfig/streamline_config.pl
> > +++ b/scripts/kconfig/streamline_config.pl
> > @@ -237,7 +237,7 @@ sub read_kconfig {
> > }
> >
> > # configs without prompts must be selected
> > - } elsif ($state ne "NONE" && /^\s*tristate\s\S/) {
> > + } elsif ($state ne "NONE" && /^\s*(tristate\s+\S|prompt\b)/) {
>
> I prefer not to have the "prompt" here. I'm only interested in module
> configs. But the \s to \s+ is a real fix.

Separate tristate and prompt statements are allowed and recognized by
the kconfig parser. Although there are no such cases now, there used to
be one: 0a57274
You sure you want to remove it from the regexp?

2016-04-26 22:34:49

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v2] localmodconfig: Fix whitespace repeat count after "tristate"

On Tue, 26 Apr 2016 14:35:19 -0700
Benjamin Poirier <[email protected]> wrote:

> Separate tristate and prompt statements are allowed and recognized by
> the kconfig parser. Although there are no such cases now, there used to
> be one: 0a57274
> You sure you want to remove it from the regexp?

Hmm, OK, I'll take your patch as is.

Thanks,

-- Steve