| 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
| ||
|
Date: Mon, 12 Dec 2022 19:19:29 +0530
From: Srinivasa Rao Mandadapu <quic_srivasam@...cinc.com>
To: Stephen Boyd <swboyd@...omium.org>, <agross@...nel.org>,
<andersson@...nel.org>, <bgoswami@...cinc.com>,
<broonie@...nel.org>, <devicetree@...r.kernel.org>,
<judyhsiao@...omium.org>, <krzysztof.kozlowski@...aro.org>,
<lgirdwood@...il.com>, <linux-arm-msm@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-remoteproc@...r.kernel.org>,
<mathieu.poirier@...aro.org>, <perex@...ex.cz>,
<quic_plai@...cinc.com>, <quic_rohkumar@...cinc.com>,
<robh+dt@...nel.org>, <srinivas.kandagatla@...aro.org>,
<tiwai@...e.com>
Subject: Re: [PATCH] remoteproc: elf_loader: Update resource table name check
On 12/10/2022 2:22 AM, Stephen Boyd wrote:
Thanks for your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2022-12-08 05:40:54)
>> On 12/7/2022 7:25 AM, Stephen Boyd wrote:
>> Thanks for Your Time Stephen!!!
>>> Quoting Srinivasa Rao Mandadapu (2022-12-01 04:20:48)
>>>> 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.
>>>>
>>> Does this need a Fixes tag?
>> I don't think so. I feel It's kind of enhancement.
>>>> 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"))
>>> Was the strcmp not working before because the 'name_table' has something
>>> else in it? It really looks like your elf file is malformed.
>> Actually, DSP binary is prepared by combining different elfs. So Section
>> header names are appended with
>>
>> something else to distinguish same section name of different elfs, by
>> using some Qualcomm specific QURT scripts.
>> Hence final binary contains resource_table name appended with module
>> specific name.
>>
>> So this patch is required to handle such modified name.
>>
> Can you clarify how the section header name looks? Probably you can
> objdump the section here and provide that information to help us
> understand.
Here is the Section header info.
$ readelf -SW bootimage_relocflag_kodiak.adsp.prodQ.pbn
There are 65 section headers, starting at offset 0x434:
readelf: Error: File contains multiple dynamic symbol tables
Section Headers:
[Nr] Name Type
[ 0] NULL
[ 1] .shstrtab STRTAB
[ 2] .text.ukernel.island PROGBITS
[ 3] .data.ukernel.island PROGBITS
[ 4] .qskernel_exports.island PROGBITS
[ 5] .start PROGBITS
[ 6] .qskernel_main PROGBITS
[ 7] .qskernel_rest PROGBITS
[ 8] .comment PROGBITS
[ 9] .data.common.island PROGBITS
[10] .rodata.common.island PROGBITS
[11] .data.va.island NOBITS
[12] .rodata.va.island PROGBITS
[13] .text.common.island PROGBITS
[14] .text.va.island PROGBITS
[15] .start PROGBITS
[16] .ukernel.main PROGBITS
[17] .init PROGBITS
[18] .text PROGBITS
[19] .fini PROGBITS
[20] .interp PROGBITS
[21] .hash HASH
[22] .dynsym DYNSYM
[23] .dynstr STRTAB
[24] .rodata PROGBITS
[25] .eh_frame PROGBITS
[26] .ctors PROGBITS
[27] .dtors PROGBITS
[28] .dynamic DYNAMIC
[29] .data PROGBITS
[30] .bss NOBITS
[31] .sdata PROGBITS
[32] QSR_STRING PROGBITS
[33] .comment PROGBITS
[34] .tcm_island.audio_process PROGBITS
[35] .data.common.island.audio_process PROGBITS
[36] .rodata.common.island.audio_process PROGBITS
[37] .data.va.island.audio_process PROGBITS
[38] .rodata.va.island.audio_process PROGBITS
[39] .text.common.island.audio_process PROGBITS
[40] .text.va.island.audio_process PROGBITS
[41] .start.audio_process PROGBITS
[42] .init.audio_process PROGBITS
[43] .text.audio_process PROGBITS
[44] .fini.audio_process PROGBITS
[45] .interp.audio_process PROGBITS
[46] .hash.audio_process HASH
[47] .dynsym.audio_process DYNSYM
[48] .dynstr.audio_process STRTAB
[49] .rodata.audio_process PROGBITS
[50] .eh_frame.audio_process PROGBITS
[51] .ctors.audio_process PROGBITS
[52] .dtors.audio_process PROGBITS
[53] .data.rel.ro.audio_process PROGBITS
[54] .dynamic.audio_process DYNAMIC
[55] .data.audio_process PROGBITS
[56] .bss.audio_process NOBITS
[57] .sdata.audio_process PROGBITS
[58] QSR_STRING.audio_process PROGBITS
[59] .comment.audio_process PROGBITS
[60] .start.ac_bin_process PROGBITS
[61] .resource_table.ac_bin_process PROGBITS
[62] .comment.ac_bin_process PROGBITS
>
> I think remoteproc_elf_loader.c assumes the ELF file is properly formed.
> There should be a section named '.resource_table', so the strcmp will
> find it by looking at the section header names.
Powered by blists - more mailing lists