linux-kernel - Re: [PATCH v5 1/3] xhci: hub: export symbol on xhci_hub

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:   Fri, 28 Oct 2022 12:37:15 +0200
From:   "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
To:     Thierry Reding <thierry.reding@...il.com>
Cc:     Jim Lin <jilin@...dia.com>, Jonathan Hunter <jonathanh@...dia.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        "mathias.nyman@...el.com" <mathias.nyman@...el.com>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
Subject: Re: [PATCH v5 1/3] xhci: hub: export symbol on xhci_hub_control

On Fri, Oct 28, 2022 at 11:50:47AM +0200, Thierry Reding wrote:
> On Fri, Oct 28, 2022 at 08:04:54AM +0200, gregkh@...uxfoundation.org wrote:
> > On Fri, Oct 28, 2022 at 05:36:41AM +0000, Jim Lin wrote:
> > > On Thu, 2022-10-27 at 15:58 +0200, Greg KH wrote:
> > > > External email: Use caution opening links or attachments
> > > > 
> > > > 
> > > > On Thu, Oct 27, 2022 at 09:31:25PM +0800, Jim Lin wrote:
> > > > > Add EXPORT_SYMBOL_GPL on xhci_hub_control() for other driver module
> > > > > to invoke and avoid linking error.
> > > > 
> > > > What other driver module?
> > > > 
> > > > There is no user here :(
> > > > 
> > > > confused,
> > > > 
> > > 
> > > In arch/arm/configs/multi_v7_defconfig
> > > It defines
> > > CONFIG_USB_XHCI_TEGRA=m
> > > 
> > > If I don't add EXPORT_SYMBOL_GPL on xhci_hub_control()
> > > , I will get compile/linking error like
> > > 
> > > ERROR: modpost: "xhci_hub_control" [drivers/usb/host/xhci-tegra.ko]
> > > undefined!
> > > 
> > > if patch
> > > "[PATCH v5,3/3] xhci: tegra: USB2 pad power controls"
> > > 
> > > https://patchwork.kernel.org/project/linux-usb/patch/20221027133127.27592-4-jilin@nvidia.com/
> > > is added in xhci-tegra.c to invoke xhci_hub_control()
> > > 
> > > Should I integrate this patch with [PATCH v5,3/3] as one?
> > 
> > Yes, do not add something that is not needed for that specific commit,
> > otherwise it causes reviewers to be confused.
> 
> Other subsystem maintainers prefer core changes to be split from driver
> changes, so this type of split is commonly encountered elsewhere.
> 
> Obviously, since this is your turf you get to make the rules. I'm just
> trying to say that this kind of advice can be confusing for contributors
> because when they then sent driver and code changes mixed for their next
> submission, the subsystem maintainer might tell them otherwise.

Sure, but if you do split it up like this, DOCUMENT WHY THE EXPORT IS
NEEDED.  That didn't happen here so I had no idea why this was even an
issue.

And yes, I am very sensitive to this, we have had LOTS of people trying
to export xhci symbols in the past few years for no in-kernel users,
despite us constantly telling them that this is not allowed.  It
happened again, just yesterday:
	https://lore.kernel.org/r/20221027004050.4192111-1-albertccwang@google.com

And at first glance, I assumed this was much the same as there was no
description of why this was needed at all.

thanks,

greg k-h