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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 26 Sep 2019 08:03:01 +0000
From:   Anson Huang <anson.huang@....com>
To:     Marco Felsch <m.felsch@...gutronix.de>
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, 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.

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!

Anson