linux-kernel - Re: [PATCH v2 1/4] mmc: sdhci-of-aspeed: Fix link failure for SPARC

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] [day] [month] [year] [list]

Message-Id: <38ff59e7-491b-478b-afff-a664d8f66547@www.fastmail.com>
Date:   Wed, 04 Sep 2019 09:32:44 +0930
From:   "Andrew Jeffery" <andrew@...id.au>
To:     "Ulf Hansson" <ulf.hansson@...aro.org>
Cc:     "Joel Stanley" <joel@....id.au>, "Arnd Bergmann" <arnd@...db.de>,
        linux-mmc <linux-mmc@...r.kernel.org>,
        "Adrian Hunter" <adrian.hunter@...el.com>,
        "OpenBMC Maillist" <openbmc@...ts.ozlabs.org>,
        "Linux ARM" <linux-arm-kernel@...ts.infradead.org>,
        linux-aspeed <linux-aspeed@...ts.ozlabs.org>,
        "Linux Kernel Mailing List" <linux-kernel@...r.kernel.org>,
        "kbuild test robot" <lkp@...el.com>
Subject: Re: [PATCH v2 1/4] mmc: sdhci-of-aspeed: Fix link failure for SPARC



On Wed, 4 Sep 2019, at 00:18, Ulf Hansson wrote:
> On Mon, 2 Sep 2019 at 07:26, Andrew Jeffery <andrew@...id.au> wrote:
> >
> >
> >
> > On Mon, 2 Sep 2019, at 13:42, Joel Stanley wrote:
> > > On Mon, 2 Sep 2019 at 03:58, Andrew Jeffery <andrew@...id.au> wrote:
> > > >
> > > > Resolves the following build error reported by the 0-day bot:
> > > >
> > > >     ERROR: "of_platform_device_create" [drivers/mmc/host/sdhci-of-aspeed.ko] undefined!
> > > >
> > > > SPARC does not set CONFIG_OF_ADDRESS so the symbol is missing. Guard the
> > > > callsite to maintain build coverage for the rest of the driver.
> > > >
> > > > Reported-by: kbuild test robot <lkp@...el.com>
> > > > Signed-off-by: Andrew Jeffery <andrew@...id.au>
> > > > ---
> > > >  drivers/mmc/host/sdhci-of-aspeed.c | 38 ++++++++++++++++++++----------
> > > >  1 file changed, 25 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
> > > > index d5acb5afc50f..96ca494752c5 100644
> > > > --- a/drivers/mmc/host/sdhci-of-aspeed.c
> > > > +++ b/drivers/mmc/host/sdhci-of-aspeed.c
> > > > @@ -224,10 +224,30 @@ static struct platform_driver aspeed_sdhci_driver = {
> > > >         .remove         = aspeed_sdhci_remove,
> > > >  };
> > > >
> > > > -static int aspeed_sdc_probe(struct platform_device *pdev)
> > > > -
> > > > +static int aspeed_sdc_create_sdhcis(struct platform_device *pdev)
> > > >  {
> > > > +#if defined(CONFIG_OF_ADDRESS)
> > >
> > > This is going to be untested code forever, as no one will be running
> > > on a chip with this hardware present but OF_ADDRESS disabled.
> > >
> > > How about we make the driver depend on OF_ADDRESS instead?
> >
> > Testing is split into two pieces here: compile-time and run-time.
> > Clearly the run-time behaviour is going to be broken on configurations
> > without CONFIG_OF_ADDRESS (SPARC as mentioned), but I don't think
> > that means we shouldn't allow it to be compiled in that case
> > (e.g. CONFIG_COMPILE_TEST performs a similar role).
> >
> > With respect to compile-time it's possible to compile either path as
> > demonstrated by the build failure report.
> >
> > Having said that there's no reason we  couldn't do what you suggest,
> > just it wasn't the existing solution pattern for the problem (there are
> > several other drivers that suffered the same bug that were fixed in the
> > style of this patch). Either way works, it's all somewhat academic.
> > Your suggestion is more obvious in terms of correctness, but this
> > patch is basically just code motion (the only addition is the `#if`/
> > `#endif` lines over what was already there if we disregard the
> > function declaration/invocation). I'll change it if there are further
> > complaints and a reason to do a v3.
> 
> I am in favor of Joel's suggestion as I don't really like having
> ifdefs bloating around in the driver (unless very good reasons).
> Please re-spin a v3.
> 
> Another option is to implement stub function and to deal with error
> codes, but that sounds more like a long term thingy, if even
> applicable here.

No worries then, will post a respin shortly.

Andrew