Re: [PATCH 1/2] soc: qcom: smem: add boundary checks for partitions

[Albert Esteve]

On Wed, Sep 30, 2026 at 7:11 PM Deepak Kumar Singh
<deepak.singh@xxxxxxxxxxxxxxxx> wrote:
>
>
> On 9/1/2026 12:34 PM, Albert Esteve wrote:
> > On Tue, Sep 1, 2026 at 12:54 AM Bjorn Andersson <andersson@xxxxxxxxxx> wrote:
> >> On Thu, Aug 20, 2026 at 01:34:18PM +0200, Albert Esteve wrote:
> >>> From: Sudeepgoud Patil <quic_sudeepgo@xxxxxxxxxxx>
> >>>
> >>> Add condition check to make sure that the end address
> >>> of private entry does not go out of partition.
> >>>
> >>> Signed-off-by: Sarannya S <quic_sarannya@xxxxxxxxxxx>
> >>> Signed-off-by: Pranav Mahesh Phansalkar <quic_pphansal@xxxxxxxxxxx>
> >>> Signed-off-by: Sudeepgoud Patil <quic_sudeepgo@xxxxxxxxxxx>
> >>> Signed-off-by: Albert Esteve <aesteve@xxxxxxxxxx>
> >> I'm sorry, Albert. Sudeepgoud messed up the authorship vs signed-off-by
> >> chain here. The author should be "Sarannya", as that's the first
> >> signed-off-by.
> >>
> >> PS. It would also be nice if this was submitted with updated email
> >> addresses and copyright statement. But I'd not expect you to fix that.
> > Hi Bjorn,
> >
> > I'm happy to address these changes for V2. Do you know where to find
> > the updated email addresses?
> >
> > BR,
> > Albert.
>
> Hi Albert,
>
> Sorry for providing this info late. In case you post new series please use below ids-
>
> Sarannya S <sarannya.s@xxxxxxxxxxxxxxxx>
> Pranav Mahesh Phansalkar <pranav.phansalkar@xxxxxxxxxxxxxxxx>
> Tony Truong <tony.truong@xxxxxxxxxxxxxxxx>
> Sudeepgoud Patil <sudeep.patil@xxxxxxxxxxxxxxxx>

Thanks. Series is currently in its v3
(https://lore.kernel.org/all/20260924-port-smem-v3-0-fbc2ea81a654@xxxxxxxxxx/).
I got the domains correct at least :)

If a v4 is needed, I will use these addresses.

BR,
Albert.

>
> Regards,
> Deepak
>
> >
> >> Regards,
> >> Bjorn
> >>
> >>> ---
> >>>   drivers/soc/qcom/smem.c | 105 +++++++++++++++++++++++++++++++++---------------
> >>>   1 file changed, 72 insertions(+), 33 deletions(-)
> >>>
> >>> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
> >>> index afb21a778fe7b..194ffb2ac010f 100644
> >>> --- a/drivers/soc/qcom/smem.c
> >>> +++ b/drivers/soc/qcom/smem.c
> >>> @@ -2,6 +2,7 @@
> >>>   /*
> >>>    * Copyright (c) 2015, Sony Mobile Communications AB.
> >>>    * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
> >>> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
> >>>    */
> >>>
> >>>   #include <linux/hwspinlock.h>
> >>> @@ -85,6 +86,17 @@
> >>>   /* Processor/host identifier for the global partition */
> >>>   #define SMEM_GLOBAL_HOST     0xfffe
> >>>
> >>> +/* Entry range check
> >>> + * ptr >= start : Checks if ptr is greater than the start of access region
> >>> + * ptr + size >= ptr: Check for integer overflow (On 32bit system where ptr
> >>> + * and size are 32bits, ptr + size can wrap around to be a small integer)
> >>> + * ptr + size <= end: Checks if ptr+size is less than the end of access region
> >>> + */
> >>> +#define IN_PARTITION_RANGE(ptr, size, start, end)            \
> >>> +     (((void *)(ptr) >= (void *)(start)) &&                  \
> >>> +      (((void *)(ptr) + (size)) >= (void *)(ptr)) && \
> >>> +      (((void *)(ptr) + (size)) <= (void *)(end)))
> >>> +
> >>>   /**
> >>>     * struct smem_proc_comm - proc_comm communication struct (legacy)
> >>>     * @command:        current command to be executed
> >>> @@ -403,6 +415,7 @@ static int qcom_smem_alloc_private(struct qcom_smem *smem,
> >>>                                   size_t size)
> >>>   {
> >>>        struct smem_private_entry *hdr, *end;
> >>> +     struct smem_private_entry *next_hdr;
> >>>        struct smem_partition_header *phdr;
> >>>        size_t alloc_size;
> >>>        void *cached;
> >>> @@ -415,19 +428,25 @@ static int qcom_smem_alloc_private(struct qcom_smem *smem,
> >>>        end = phdr_to_last_uncached_entry(phdr);
> >>>        cached = phdr_to_last_cached_entry(phdr);
> >>>
> >>> -     if (WARN_ON((void *)end > p_end || cached > p_end))
> >>> +     if (WARN_ON(!IN_PARTITION_RANGE(end, 0, phdr, cached) ||
> >>> +                                             cached > p_end))
> >>>                return -EINVAL;
> >>>
> >>> -     while (hdr < end) {
> >>> +     while ((hdr < end) && ((hdr + 1) < end)) {
> >>>                if (hdr->canary != SMEM_PRIVATE_CANARY)
> >>>                        goto bad_canary;
> >>>                if (le16_to_cpu(hdr->item) == item)
> >>>                        return -EEXIST;
> >>>
> >>> -             hdr = uncached_entry_next(hdr);
> >>> +             next_hdr = uncached_entry_next(hdr);
> >>> +
> >>> +             if (WARN_ON(next_hdr <= hdr))
> >>> +                     return -EINVAL;
> >>> +
> >>> +             hdr = next_hdr;
> >>>        }
> >>>
> >>> -     if (WARN_ON((void *)hdr > p_end))
> >>> +     if (WARN_ON((void *)hdr > (void *)end))
> >>>                return -EINVAL;
> >>>
> >>>        /* Check that we don't grow into the cached region */
> >>> @@ -587,9 +606,11 @@ static void *qcom_smem_get_private(struct qcom_smem *smem,
> >>>                                   unsigned item,
> >>>                                   size_t *size)
> >>>   {
> >>> -     struct smem_private_entry *e, *end;
> >>> +     struct smem_private_entry *e, *uncached_end, *cached_end;
> >>> +     struct smem_private_entry *next_e;
> >>>        struct smem_partition_header *phdr;
> >>>        void *item_ptr, *p_end;
> >>> +     size_t entry_size = 0;
> >>>        u32 padding_data;
> >>>        u32 e_size;
> >>>
> >>> @@ -597,67 +618,85 @@ static void *qcom_smem_get_private(struct qcom_smem *smem,
> >>>        p_end = (void *)phdr + part->size;
> >>>
> >>>        e = phdr_to_first_uncached_entry(phdr);
> >>> -     end = phdr_to_last_uncached_entry(phdr);
> >>> +     uncached_end = phdr_to_last_uncached_entry(phdr);
> >>> +     cached_end = phdr_to_last_cached_entry(phdr);
> >>> +
> >>> +     if (WARN_ON(!IN_PARTITION_RANGE(uncached_end, 0, phdr, cached_end)
> >>> +                                     || (void *)cached_end > p_end))
> >>> +             return ERR_PTR(-EINVAL);
> >>>
> >>> -     while (e < end) {
> >>> +     while ((e < uncached_end) && ((e + 1) < uncached_end)) {
> >>>                if (e->canary != SMEM_PRIVATE_CANARY)
> >>>                        goto invalid_canary;
> >>>
> >>>                if (le16_to_cpu(e->item) == item) {
> >>> -                     if (size != NULL) {
> >>> -                             e_size = le32_to_cpu(e->size);
> >>> -                             padding_data = le16_to_cpu(e->padding_data);
> >>> +                     e_size = le32_to_cpu(e->size);
> >>> +                     padding_data = le16_to_cpu(e->padding_data);
> >>>
> >>> -                             if (WARN_ON(e_size > part->size || padding_data > e_size))
> >>> -                                     return ERR_PTR(-EINVAL);
> >>> +                     if (e_size < part->size && padding_data < e_size)
> >>> +                             entry_size = e_size - padding_data;
> >>> +                     else
> >>> +                             return ERR_PTR(-EINVAL);
> >>>
> >>> -                             *size = e_size - padding_data;
> >>> -                     }
> >>> +                     item_ptr =  uncached_entry_to_item(e);
> >>>
> >>> -                     item_ptr = uncached_entry_to_item(e);
> >>> -                     if (WARN_ON(item_ptr > p_end))
> >>> +                     if (WARN_ON(!IN_PARTITION_RANGE(item_ptr, entry_size, e, uncached_end)))
> >>>                                return ERR_PTR(-EINVAL);
> >>>
> >>> +                     if (size != NULL)
> >>> +                             *size = entry_size;
> >>> +
> >>>                        return item_ptr;
> >>>                }
> >>>
> >>> -             e = uncached_entry_next(e);
> >>> -     }
> >>> +             next_e = uncached_entry_next(e);
> >>> +             if (WARN_ON(next_e <= e))
> >>> +                     return ERR_PTR(-EINVAL);
> >>>
> >>> -     if (WARN_ON((void *)e > p_end))
> >>> +             e = next_e;
> >>> +     }
> >>> +     if (WARN_ON((void *)e > (void *)uncached_end))
> >>>                return ERR_PTR(-EINVAL);
> >>>
> >>>        /* Item was not found in the uncached list, search the cached list */
> >>>
> >>> +     if (cached_end == p_end)
> >>> +             return ERR_PTR(-ENOENT);
> >>> +
> >>>        e = phdr_to_first_cached_entry(phdr, part->cacheline);
> >>> -     end = phdr_to_last_cached_entry(phdr);
> >>>
> >>> -     if (WARN_ON((void *)e < (void *)phdr || (void *)end > p_end))
> >>> +     if (WARN_ON(!IN_PARTITION_RANGE(cached_end, 0, uncached_end, p_end) ||
> >>> +                     !IN_PARTITION_RANGE(e, sizeof(*e), cached_end, p_end)))
> >>>                return ERR_PTR(-EINVAL);
> >>>
> >>> -     while (e > end) {
> >>> +     while (e > cached_end) {
> >>>                if (e->canary != SMEM_PRIVATE_CANARY)
> >>>                        goto invalid_canary;
> >>>
> >>>                if (le16_to_cpu(e->item) == item) {
> >>> -                     if (size != NULL) {
> >>> -                             e_size = le32_to_cpu(e->size);
> >>> -                             padding_data = le16_to_cpu(e->padding_data);
> >>> +                     e_size = le32_to_cpu(e->size);
> >>> +                     padding_data = le16_to_cpu(e->padding_data);
> >>>
> >>> -                             if (WARN_ON(e_size > part->size || padding_data > e_size))
> >>> -                                     return ERR_PTR(-EINVAL);
> >>> -
> >>> -                             *size = e_size - padding_data;
> >>> -                     }
> >>> +                     if (e_size < part->size && padding_data < e_size)
> >>> +                             entry_size  = e_size - padding_data;
> >>> +                     else
> >>> +                             return ERR_PTR(-EINVAL);
> >>>
> >>> -                     item_ptr = cached_entry_to_item(e);
> >>> -                     if (WARN_ON(item_ptr < (void *)phdr))
> >>> +                     item_ptr =  cached_entry_to_item(e);
> >>> +                     if (WARN_ON(!IN_PARTITION_RANGE(item_ptr, entry_size, cached_end, e)))
> >>>                                return ERR_PTR(-EINVAL);
> >>>
> >>> +                     if (size != NULL)
> >>> +                             *size = entry_size;
> >>> +
> >>>                        return item_ptr;
> >>>                }
> >>>
> >>> -             e = cached_entry_next(e, part->cacheline);
> >>> +             next_e = cached_entry_next(e, part->cacheline);
> >>> +             if (WARN_ON(next_e >= e))
> >>> +                     return ERR_PTR(-EINVAL);
> >>> +
> >>> +             e = next_e;
> >>>        }
> >>>
> >>>        if (WARN_ON((void *)e < (void *)phdr))
> >>>
> >>> --
> >>> 2.55.0
> >>>
>