Re: [PATCH] remoteproc: elf_loader: Update resource table name check
From: Srinivasa Rao Mandadapu
Date: Mon Dec 12 2022 - 08:51:41 EST
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@xxxxxxxxxxx>
---
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.