linux-kernel - Re: [PATCH v3 2/2] PCI: qcom: Add support for modular builds

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:   Tue, 18 Oct 2022 11:08:59 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Johan Hovold <johan@...nel.org>
Cc:     Johan Hovold <johan+linaro@...nel.org>,
        Stanimir Varbanov <svarbanov@...sol.com>,
        Lorenzo Pieralisi <lpieralisi@...nel.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Manivannan Sadhasivam <mani@...nel.org>,
        linux-arm-msm@...r.kernel.org, linux-pci@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Rob Herring <robh@...nel.org>,
        Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Subject: Re: [PATCH v3 2/2] PCI: qcom: Add support for modular builds

On Tue, Oct 18, 2022 at 05:10:53PM +0200, Johan Hovold wrote:
> On Mon, Oct 17, 2022 at 12:34:22PM -0500, Bjorn Helgaas wrote:
> > On Mon, Oct 17, 2022 at 01:47:05PM +0200, Johan Hovold wrote:
> > > Allow the Qualcomm PCIe controller driver to be built as a module, which
> > > is useful for multi-platform kernels as well as during development.
> > 
> > There are two different goals here, and there's no real reason to
> > bundle them together:
> > 
> >   1) Make qcom a loadable module.  This is a hard requirement so
> >      multi-platform kernels don't need to build in all drivers
> >      statically.
> > 
> >   2) Make qcom unloadable.  This is a high want, possibly even a
> >      requirement for developers, but is not really a big issue for
> >      users.
> > 
> > There are different changes required: 1) requires the Kconfig change;
> > 2) requires .remove() to be implemented.  Since there's no requirement
> > that these be done together, let's split them into separate patches.
> > 
> > Then we can make sure that at least 1) gets done, and if for any
> > reason 2) isn't safe or breaks something, we can at least bisect and
> > if necessary revert it without losing 1).
> 
> Implementing 1) in itself requires more than simply splitting this
> patch. And I don't think we should be making life harder for developers,
> as well as users assisting during debugging, by going in that direction.

If you're saying this patch can't be split, can you elaborate on the
details of *why* it can't be split?

> We have tons of modules in the kernel and very few that cannot be
> unloaded. Anyone who doesn't trust root to not unload modules can
> always disable unloading completely using CONFIG_MODULE_UNLOAD.

This is all true, but IIUC, the issue is about unloading IRQ
controller drivers, and this doesn't address that.  I don't have a
clear understanding of the issue, and it would be nice if a patch that
specifically added unloadability could elaborate on that.  Then we can
decide that "yes, this is a risk, and we're willing to accept it." An
argument that "tons of modules do this" totally avoids the issues of
this particular case.

Bjorn