diff mbox series

cfg80211: allow to build without CFG80211_REQUIRE_SIGNED_REGDB

Message ID 1533898547-14449-1-git-send-email-sgruszka@redhat.com (mailing list archive)
State Rejected
Delegated to: Johannes Berg
Headers show
Series cfg80211: allow to build without CFG80211_REQUIRE_SIGNED_REGDB | expand

Commit Message

Stanislaw Gruszka Aug. 10, 2018, 10:55 a.m. UTC
According to kconfig-language.txt conditional dependency should be
expressed 2 times:

        bool "foo" if BAR
        default y if BAR

Indeed, without additional if expression we always build with
CFG80211_REQUIRE_SIGNED_REGDB even when CFG80211_CERTIFICATION_ONUS
is not set.

Fixes: 90a53e4432b1 ("cfg80211: implement regdb signature checking")
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 net/wireless/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Johannes Berg Aug. 13, 2018, 10:09 a.m. UTC | #1
On Fri, 2018-08-10 at 12:55 +0200, Stanislaw Gruszka wrote:
> According to kconfig-language.txt conditional dependency should be
> expressed 2 times:
> 
>         bool "foo" if BAR
>         default y if BAR
> 
> Indeed, without additional if expression we always build with
> CFG80211_REQUIRE_SIGNED_REGDB even when CFG80211_CERTIFICATION_ONUS
> is not set.

That's the intent. If you do set CERTIFICATION_ONUS, then you can
disable this (presumably because you have external OS image verification
mechanisms, or similar).

If you don't set CERTIFICATION_ONUS, this should always be set.

Perhaps it should be renamed to CFG80211_REQUIRE_REGDB_SIGNATURE or so,
which might be clearer? And a case has been made before for adding
CFG80211_FIRMWARE_REGDB_SUPPORT that controls the whole feature, but
this patch is clearly wrong.

johannes
Stanislaw Gruszka Aug. 13, 2018, 11:44 a.m. UTC | #2
On Mon, Aug 13, 2018 at 12:09:13PM +0200, Johannes Berg wrote:
> On Fri, 2018-08-10 at 12:55 +0200, Stanislaw Gruszka wrote:
> > According to kconfig-language.txt conditional dependency should be
> > expressed 2 times:
> > 
> >         bool "foo" if BAR
> >         default y if BAR
> > 
> > Indeed, without additional if expression we always build with
> > CFG80211_REQUIRE_SIGNED_REGDB even when CFG80211_CERTIFICATION_ONUS
> > is not set.

Err, I meant "is set"

> That's the intent. If you do set CERTIFICATION_ONUS, then you can
> disable this (presumably because you have external OS image verification
> mechanisms, or similar).
> 
> If you don't set CERTIFICATION_ONUS, this should always be set.

Patch allow to build without CFG80211_REQUIRE_SIGNED_REGDB. This option
is not configurable (allways y) no matter of CERTIFICATION_ONUS setting.
With the patch and with CERTIFICATION_ONUS,
CFG80211_REQUIRE_SIGNED_REGDB is still default y, but can be set to n
during "make oldconfig".

> Perhaps it should be renamed to CFG80211_REQUIRE_REGDB_SIGNATURE or so,
> which might be clearer? And a case has been made before for adding
> CFG80211_FIRMWARE_REGDB_SUPPORT that controls the whole feature, but
> this patch is clearly wrong.

Patch is fine, there is just typo in the changelog :-)

Cheers
Stanislaw
Johannes Berg Aug. 13, 2018, 11:47 a.m. UTC | #3
On Mon, 2018-08-13 at 13:44 +0200, Stanislaw Gruszka wrote:
> On Mon, Aug 13, 2018 at 12:09:13PM +0200, Johannes Berg wrote:
> > On Fri, 2018-08-10 at 12:55 +0200, Stanislaw Gruszka wrote:
> > > According to kconfig-language.txt conditional dependency should be
> > > expressed 2 times:
> > > 
> > >         bool "foo" if BAR
> > >         default y if BAR
> > > 
> > > Indeed, without additional if expression we always build with
> > > CFG80211_REQUIRE_SIGNED_REGDB even when CFG80211_CERTIFICATION_ONUS
> > > is not set.
> 
> Err, I meant "is set"

Ok, but still?

> > That's the intent. If you do set CERTIFICATION_ONUS, then you can
> > disable this (presumably because you have external OS image verification
> > mechanisms, or similar).
> > 
> > If you don't set CERTIFICATION_ONUS, this should always be set.
> 
> Patch allow to build without CFG80211_REQUIRE_SIGNED_REGDB. This option
> is not configurable (allways y) no matter of CERTIFICATION_ONUS setting.

How so? The default is y, but if CERTIFICATION_ONUS is set, you should
be able to change it.

> With the patch and with CERTIFICATION_ONUS,
> CFG80211_REQUIRE_SIGNED_REGDB is still default y, but can be set to n
> during "make oldconfig".

I don't think your patch changes anything there since it just changes
when the default is applied.

> > Perhaps it should be renamed to CFG80211_REQUIRE_REGDB_SIGNATURE or so,
> > which might be clearer? And a case has been made before for adding
> > CFG80211_FIRMWARE_REGDB_SUPPORT that controls the whole feature, but
> > this patch is clearly wrong.
> 
> Patch is fine, there is just typo in the changelog :-)

Disagree, if anything should be changed, it should be changed to

  default y if !CERTIFICATION_ONUS

but I prefer the way it works now, since it means setting certification
onus won't immediately change this setting.

johannes
Stanislaw Gruszka Aug. 13, 2018, 12:17 p.m. UTC | #4
On Mon, Aug 13, 2018 at 01:47:53PM +0200, Johannes Berg wrote:
> Disagree, if anything should be changed, it should be changed to
> 
>   default y if !CERTIFICATION_ONUS
> 
> but I prefer the way it works now, since it means setting certification
> onus won't immediately change this setting.

Ok, this works as supposed. Not sure why it did not work for me
before, maybe I just confused config options.

Regards
Stanislaw
diff mbox series

Patch

diff --git a/net/wireless/Kconfig b/net/wireless/Kconfig
index 4172204..bd63b73 100644
--- a/net/wireless/Kconfig
+++ b/net/wireless/Kconfig
@@ -89,7 +89,7 @@  config CFG80211_CERTIFICATION_ONUS
 
 config CFG80211_REQUIRE_SIGNED_REGDB
 	bool "require regdb signature" if CFG80211_CERTIFICATION_ONUS
-	default y
+	default y if CFG80211_CERTIFICATION_ONUS
 	select SYSTEM_DATA_VERIFICATION
 	help
 	  Require that in addition to the "regulatory.db" file a