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

[Konrad Dybcio]

On 10/22/26 11:47 AM, Albert Esteve wrote:
> From: Sarannya S <sarannya.s@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 <sarannya.s@xxxxxxxxxxxxxxxx>
> Signed-off-by: Pranav Mahesh Phansalkar <pranav.phansalkar@xxxxxxxxxxxxxxxx>
> Signed-off-by: Sudeepgoud Patil <sudeep.patil@xxxxxxxxxxxxxxxx>
> Signed-off-by: Albert Esteve <aesteve@xxxxxxxxxx>
> ---
>  drivers/soc/qcom/smem.c | 52 ++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 38 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
> index 36a4a10f3aec..a6732832e2c5 100644
> --- a/drivers/soc/qcom/smem.c
> +++ b/drivers/soc/qcom/smem.c
> @@ -422,6 +422,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;
> @@ -434,19 +435,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))

nit: i wouldn't break this line, and otherwise the next line should
start below where the first condition starts

[...]

> -	if (WARN_ON((void *)hdr > p_end))
> +	if (WARN_ON((void *)hdr > (void *)end))

LLM noticed that before this patch, we risk overrunning end, but with
this patch, we now risk a corrupted/malicious partition ending before
'end'. A valid walk always lands exactly on `end`, so the fix is:

if (WARN_ON(hdr != end))
    return -EINVAL;


Otherwise my clanker is reasonably happy and I see nothing wrong

Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>

Konrad