On Thu, Jun 05, 2025 at 08:43:00AM -0500, Bjorn Andersson wrote:
> When the MDT loader is used in remoteproc, the ELF header is sanitized
> beforehand, but that's not necessary the case for other clients.
>
> Validate the size of the firmware buffer to ensure that we don't read
> past the end as we iterate over the header.
>
> Fixes: 2aad40d911ee ("remoteproc: Move qcom_mdt_loader into drivers/soc/qcom")
> Cc: <[email protected]>
> Reported-by: Doug Anderson <[email protected]>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
> drivers/soc/qcom/mdt_loader.c | 37 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
> index
> b2c0fb55d4ae678ee333f0d6b8b586de319f53b1..1da22b23d19d28678ec78cccdf8c328b50d3ffda
> 100644
> --- a/drivers/soc/qcom/mdt_loader.c
> +++ b/drivers/soc/qcom/mdt_loader.c
> @@ -18,6 +18,31 @@
> #include <linux/slab.h>
> #include <linux/soc/qcom/mdt_loader.h>
>
> +static bool mdt_header_valid(const struct firmware *fw)
> +{
> + const struct elf32_hdr *ehdr;
> + size_t phend;
> + size_t shend;
> +
> + if (fw->size < sizeof(*ehdr))
> + return false;
> +
> + ehdr = (struct elf32_hdr *)fw->data;
> +
> + if (memcmp(ehdr->e_ident, ELFMAG, SELFMAG))
> + return false;
> +
> + phend = size_add(size_mul(sizeof(struct elf32_phdr), ehdr->e_phnum),
> ehdr->e_phoff);
Nit, this should be a max(sizeof() and ehdr->e_phentsize.
> + if (phend > fw->size)
> + return false;
> +
> + shend = size_add(size_mul(sizeof(struct elf32_shdr), ehdr->e_shnum),
> ehdr->e_shoff);
> + if (shend > fw->size)
Same for e_shentsize
> + return false;
> +
> + return true;
> +}
> +
> static bool mdt_phdr_valid(const struct elf32_phdr *phdr)
> {
> if (phdr->p_type != PT_LOAD)
> @@ -82,6 +107,9 @@ ssize_t qcom_mdt_get_size(const struct firmware *fw)
> phys_addr_t max_addr = 0;
> int i;
>
> + if (!mdt_header_valid(fw))
> + return -EINVAL;
> +
> ehdr = (struct elf32_hdr *)fw->data;
> phdrs = (struct elf32_phdr *)(ehdr + 1);
>
> @@ -134,6 +162,9 @@ void *qcom_mdt_read_metadata(const struct firmware *fw,
> size_t *data_len,
> ssize_t ret;
> void *data;
>
> + if (!mdt_header_valid(fw))
> + return ERR_PTR(-EINVAL);
> +
> ehdr = (struct elf32_hdr *)fw->data;
> phdrs = (struct elf32_phdr *)(ehdr + 1);
>
> @@ -214,6 +245,9 @@ int qcom_mdt_pas_init(struct device *dev, const struct
> firmware *fw,
> int ret;
> int i;
>
> + if (!mdt_header_valid(fw))
> + return -EINVAL;
> +
> ehdr = (struct elf32_hdr *)fw->data;
> phdrs = (struct elf32_phdr *)(ehdr + 1);
>
> @@ -310,6 +344,9 @@ static int __qcom_mdt_load(struct device *dev, const
> struct firmware *fw,
> if (!fw || !mem_region || !mem_phys || !mem_size)
> return -EINVAL;
>
> + if (!mdt_header_valid(fw))
> + return -EINVAL;
> +
> is_split = qcom_mdt_bins_are_split(fw, fw_name);
> ehdr = (struct elf32_hdr *)fw->data;
> phdrs = (struct elf32_phdr *)(ehdr + 1);
>
> --
> 2.49.0
>
--
With best wishes
Dmitry
