linux-kernel - Re: [PATCH] firmware: imx: Skip return value check for some special SCU firmware APIs

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 26 Sep 2019 12:05:58 +0200
From:   Marco Felsch <m.felsch@...gutronix.de>
To:     Anson Huang <anson.huang@....com>
Cc:     "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        Aisheng Dong <aisheng.dong@....com>,
        Leonard Crestez <leonard.crestez@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>
Subject: Re: [PATCH] firmware: imx: Skip return value check for some special
 SCU firmware APIs

Hi Anson,

On 19-09-26 08:03, Anson Huang wrote:
> Hi, Marco
> 
> > On 19-09-25 18:07, Anson Huang wrote:
> > > The SCU firmware does NOT always have return value stored in message
> > > header's function element even the API has response data, those
> > > special APIs are defined as void function in SCU firmware, so they
> > > should be treated as return success always.
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@....com>
> > > ---
> > > 	- This patch is based on the patch of
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> > >
> > hwork.kernel.org%2Fpatch%2F11129553%2F&amp;data=02%7C01%7Canson.
> > huang%
> > >
> > 40nxp.com%7C1f4108cc25eb4618f43c08d742576fa3%7C686ea1d3bc2b4c6fa
> > 92cd99
> > >
> > c5c301635%7C0%7C0%7C637050815608963707&amp;sdata=BZBg4cOR2rP%2
> > BRBNn15i
> > > Qq3%2FXBYwhuCLkgYzFRbfEgVU%3D&amp;reserved=0
> > > ---
> > >  drivers/firmware/imx/imx-scu.c | 34
> > > ++++++++++++++++++++++++++++++++--
> > >  1 file changed, 32 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/firmware/imx/imx-scu.c
> > > b/drivers/firmware/imx/imx-scu.c index 869be7a..ced5b12 100644
> > > --- a/drivers/firmware/imx/imx-scu.c
> > > +++ b/drivers/firmware/imx/imx-scu.c
> > > @@ -78,6 +78,11 @@ static int imx_sc_linux_errmap[IMX_SC_ERR_LAST] =
> > {
> > >  	-EIO,	 /* IMX_SC_ERR_FAIL */
> > >  };
> > >
> > > +static const struct imx_sc_rpc_msg whitelist[] = {
> > > +	{ .svc = IMX_SC_RPC_SVC_MISC, .func =
> > IMX_SC_MISC_FUNC_UNIQUE_ID },
> > > +	{ .svc = IMX_SC_RPC_SVC_MISC, .func =
> > > +IMX_SC_MISC_FUNC_GET_BUTTON_STATUS }, };
> > 
> > Is this going to be extended in the near future? I see some upcoming
> > problems here if someone uses a different scu-fw<->kernel combination as
> > nxp would suggest.
> 
> Could be, but I checked the current APIs, ONLY these 2 will be used in Linux kernel, so
> I ONLY add these 2 APIs for now.

Okay.

> However, after rethink, maybe we should add another imx_sc_rpc API for those special
> APIs? To avoid checking it for all the APIs called which may impact some performance.
> Still under discussion, if you have better idea, please advise, thanks!

Adding a special api shouldn't be the right fix. Imagine if someone (not
a nxp-developer) wants to add a new driver. How could he be expected to
know which api he should use. The better abbroach would be to fix the
scu-fw instead of adding quirks..

Regards,
  Marco


> 
> Anson