On Mon, Apr 10, 2023 at 10:14:32AM -0700, Gokul krishna Krishnakumar wrote: > It may be that the offset of the first program header lies inside the mdt's > filesize, in this case the loader would incorrectly assume that the bins > were not split. The loading would then continue on to fail for split bins. > This change updates the logic used by the mdt loader to understand whether > the firmware images are split or not. It figures this out by checking if > each programs header's segment lies within the file or not. > > Signed-off-by: Gokul krishna Krishnakumar <quic_gokukris@xxxxxxxxxxx> > Signed-off-by: Melody Olvera <quic_molvera@xxxxxxxxxxx> Please read Documentation/process/submitting-patches.rst section about Developer's Certificate of Origin 1.1, then apply that for each person in the S-o-b chain from the top. Finally consider how Melody's name can be the last one in this list, while the patch is coming from you. > --- > V5: Removes extra empty lines from V4. That's nice, but what about the other changes I asked for? Regards, Bjorn > > V4: Removes the unneceessary change in qcom_mdt_read_metadata(), the > exisiting check holds good in case the hash segment is in the loaded image. > > V3 is separated out from [1] and includes > changes addressing comments from that patch set. > > [1] https://lore.kernel.org/all/20230306231202.12223-5-quic_molvera@xxxxxxxxxxx/ > --- > drivers/soc/qcom/mdt_loader.c | 25 +++++++++++++++++++++++-- > 1 file changed, 23 insertions(+), 2 deletions(-) > > diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c > index 33dd8c315eb7..814646ce41e5 100644 > --- a/drivers/soc/qcom/mdt_loader.c > +++ b/drivers/soc/qcom/mdt_loader.c > @@ -258,6 +258,26 @@ int qcom_mdt_pas_init(struct device *dev, const struct firmware *fw, > } > EXPORT_SYMBOL_GPL(qcom_mdt_pas_init); > > +static bool qcom_mdt_bins_are_split(const struct firmware *fw, const char* fw_name) > +{ > + const struct elf32_phdr *phdrs; > + const struct elf32_hdr *ehdr; > + uint64_t seg_start, seg_end; > + int i; > + > + ehdr = (struct elf32_hdr *)fw->data; > + phdrs = (struct elf32_phdr *)(ehdr + 1); > + > + for (i = 0; i < ehdr->e_phnum; i++) { > + seg_start = phdrs[i].p_offset; > + seg_end = phdrs[i].p_offset + phdrs[i].p_filesz; > + if (seg_start > fw->size || seg_end > fw->size) > + return true; > + } > + > + return false; > +} > + > static int __qcom_mdt_load(struct device *dev, const struct firmware *fw, > const char *fw_name, int pas_id, void *mem_region, > phys_addr_t mem_phys, size_t mem_size, > @@ -270,6 +290,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw, > phys_addr_t min_addr = PHYS_ADDR_MAX; > ssize_t offset; > bool relocate = false; > + bool is_split; > void *ptr; > int ret = 0; > int i; > @@ -277,6 +298,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw, > if (!fw || !mem_region || !mem_phys || !mem_size) > return -EINVAL; > > + is_split = qcom_mdt_bins_are_split(fw, fw_name); > ehdr = (struct elf32_hdr *)fw->data; > phdrs = (struct elf32_phdr *)(ehdr + 1); > > @@ -330,8 +352,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw, > > ptr = mem_region + offset; > > - if (phdr->p_filesz && phdr->p_offset < fw->size && > - phdr->p_offset + phdr->p_filesz <= fw->size) { > + if (phdr->p_filesz && !is_split) { > /* Firmware is large enough to be non-split */ > if (phdr->p_offset + phdr->p_filesz > fw->size) { > dev_err(dev, "file %s segment %d would be truncated\n", > -- > 2.39.2 >
