Re: [PATCH v3 2/4] soc: qcom: smem: validate private entry list walks

[Albert Esteve]

On Wed, Oct 21, 2026 at 2:39 PM Konrad Dybcio
<konrad.dybcio@xxxxxxxxxxxxxxxx> wrote:
>
> On 9/24/26 3:10 PM, Albert Esteve wrote:
> > From: Sarannya S <quic_sarannya@xxxxxxxxxxxxxxxx>
> >
> > When walking private entries, use the uncached/cached free offsets as
> > the list bounds, require room for a header before dereferencing, and
> > reject a next pointer that does not advance.
> >
> > Skip the cached list when the cached free offset is at the partition
> > end.
> >
> > Signed-off-by: Sarannya S <quic_sarannya@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Pranav Mahesh Phansalkar <quic_pphansal@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Sudeepgoud Patil <quic_sudeepgo@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Albert Esteve <aesteve@xxxxxxxxxx>
> > ---
>
> [...]
>
> > -     while (hdr < end) {
> > +     while ((hdr + 1) < end) {
>
> This will reject a header that's at the tail of the buffer, need
> to make this <= instead

Roger. This is a bit hard to read as intuitively I would have thought
end is the first free byte after the last header, so `(hdr + 1) < end`
should include it? End uses `offset_free_uncached` which should be
bigger than `hdr + 1` and it would not miss the tail. But being
defensive about its value seems like the right call. I will change it.

>
> [...]
>
> > -     while (e < end) {
> > +     while ((e + 1) < uncached_end) {
>
> Likewise
>
> [...]
>
> > -             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))
>
> And LLM thinks should compare against cached_end

This is preexisting but I think the LLM is right, and it matches the
theme of the patch. I will change it.

>
> Konrad
>