linux-kernel - Re: [GIT PULL] Networking for 6.0

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]

Date:   Wed, 3 Aug 2022 17:27:07 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Pablo Neira Ayuso <pablo@...filter.org>
Cc:     Paolo Abeni <pabeni@...hat.com>, Vlad Buslov <vladbu@...dia.com>,
        Oz Shlomo <ozsh@...dia.com>, kuba@...nel.org,
        davem@...emloft.net, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] Networking for 6.0

On Wed, Aug 3, 2022 at 5:11 PM Pablo Neira Ayuso <pablo@...filter.org> wrote:
>
> For these two questions, this new Kconfig toggle was copied from:
>
>  config NF_CONNTRACK_PROCFS
>         bool "Supply CT list in procfs (OBSOLETE)"
>         default y
>         depends on PROC_FS
>
> which is under:
>
>  if NF_CONNTRACK
>
> but the copy and paste was missing this.

Note that there's two problems with that

 (1) the NF_CONNTRACK_PROCFS thing is 'default y' because it *USED* to
be unconditional, and was split up as a config option back in 2011.

See commit 54b07dca6855 ("netfilter: provide config option to disable
ancient procfs parts").

IOW, that NF_CONNTRACK_PROCFS exists and defaults to on, not because
people added new code and wanted to make it default, but because the
code used to always be enabled if NF_CONNTRACK was enabled, and people
wanted the option to *disable* it.

That's when you do 'default y' - you take existing code that didn't
originally have a question at all, and you make it optional. Then you
use 'default y' so that people who used it don't get screwed in the
process.

 (2) it didn't do the proper conditional on the feature it depended on.

So let's not do copy-and-paste programming. The old Kconfig snippet
had completely different rules, had completely different history, and
completely different default values as a result.

I realize that it's very easy to think of Kconfig as some
not-very-important detail to just hook things up. But because it's
front-facing to users, I do want people to think about it more than
perhaps people otherwise would.

                Linus