linux-kernel - Re: [PATCH] remoteproc: elf_loader: Update resource table name check

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:   Mon, 12 Dec 2022 19:57:59 +0530
From:   Srinivasa Rao Mandadapu <quic_srivasam@...cinc.com>
To:     Peng Fan <peng.fan@....nxp.com>,
        <linux-remoteproc@...r.kernel.org>, <agross@...nel.org>,
        <andersson@...nel.org>, <lgirdwood@...il.com>,
        <broonie@...nel.org>, <robh+dt@...nel.org>,
        <quic_plai@...cinc.com>, <bgoswami@...cinc.com>, <perex@...ex.cz>,
        <tiwai@...e.com>, <srinivas.kandagatla@...aro.org>,
        <quic_rohkumar@...cinc.com>, <linux-arm-msm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <swboyd@...omium.org>,
        <judyhsiao@...omium.org>, <devicetree@...r.kernel.org>,
        <krzysztof.kozlowski@...aro.org>, <mathieu.poirier@...aro.org>
Subject: Re: [PATCH] remoteproc: elf_loader: Update resource table name check


On 12/10/2022 8:36 AM, Peng Fan wrote:
Thanks for your time and valuable suggestion Pang!!!
>
> On 12/1/2022 8:20 PM, Srinivasa Rao Mandadapu wrote:
>> Update resource table name check with sub string search instead of
>> complete string search.
>> In general Qualcomm binary contains, section header name
>> (e.g. .resource_table), amended with extra string to differentiate
>> with other sections.
>> So far Android adsp binaries are being authenticated using TZ,
>> hence this mismatch hasn't created any problem.
>> In recent developments, ADSP binary is being used in Chrome based
>> platforms, which doesn't have TZ path, hence resource table is
>> required for memory sandboxing.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@...cinc.com>
>> ---
>>   drivers/remoteproc/remoteproc_elf_loader.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/remoteproc/remoteproc_elf_loader.c 
>> b/drivers/remoteproc/remoteproc_elf_loader.c
>> index 5a412d7..0feb120 100644
>> --- a/drivers/remoteproc/remoteproc_elf_loader.c
>> +++ b/drivers/remoteproc/remoteproc_elf_loader.c
>> @@ -272,7 +272,7 @@ find_table(struct device *dev, const struct 
>> firmware *fw)
>>           u64 offset = elf_shdr_get_sh_offset(class, shdr);
>>           u32 name = elf_shdr_get_sh_name(class, shdr);
>>   -        if (strcmp(name_table + name, ".resource_table"))
>> +        if (!strstr(name_table + name, ".resource_table"))
>>               continue;
>
> How about ?
>     if (strcmp(name_table + name, ".resource_table")) {
>         if (strstr(name_table + name, ".resource_table"))
>               break;
>         continue;
>     }

It looks valid approach to me. But other reviewers may object on double 
check. Let's see Stephen Boyd comments also,

and handle it accordingly.

>
> Regards,
> Peng.
>>             table = (struct resource_table *)(elf_data + offset);