linux-kernel - Re: [PATCH] qcom: smem: remove hwspinlock from item get routine

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <065cbefc-031e-7595-9d48-f869585dfcd3@quicinc.com>
Date: Fri, 26 Jan 2024 16:23:43 -0800
From: Chris Lew <quic_clew@...cinc.com>
To: Tao Zhang <quic_taozhan@...cinc.com>, <andersson@...nel.org>,
        <konrad.dybcio@...aro.org>
CC: <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <kernel@...cinc.com>
Subject: Re: [PATCH] qcom: smem: remove hwspinlock from item get routine



On 1/1/2024 6:25 PM, Tao Zhang wrote:
> During an SSR(Sub-System Restart) process, the remoteproc driver will
> try to read the crash reason from SMEM. The qcom_smem_get() backing such
> operations does however take the hwspinlock (tcsr mutex), which might be
> held by the dying remoteproc.
> 
> The associated timeout on the hwspin_lock_timeout_irqsave() would take
> care of the system not hanging forever, but the get operation will fail,
> unnecessarily delaying the process for the 'HWSPINLOCK_TIMEOUT' duration
> (currently is '1s'), and finally resulting in failure to get crash
> information from SMEM.
> 
> This timeout can be avoided by removing the hwspinlock in the
> qcom_smem_get routine. SMEM ensures that the allocated item will only be
> visible after the new item is safe to use by following a specific order
> of updates.
> 
> In the private partition case, qcom_smem_get_private() will use
> 'offset_free_uncached' as a loop boundary when looking for existing
> allocated items. The corresponding allocation will only update
> offset_free_uncached once the item is fully initialized.
> 
>      hdr->canary = SMEM_PRIVATE_CANARY;
>      hdr->item = cpu_to_le16(item);
>      hdr->size = cpu_to_le32(ALIGN(size, 8));
>      hdr->padding_data = cpu_to_le16(le32_to_cpu(hdr->size) - size);
>      hdr->padding_hdr = 0;
> 
>      wmb();
>      le32_add_cpu(&phdr->offset_free_uncached, alloc_size);
> 
> The global partition is similar but uses the "entry->allocated" variable
> to ensure the item is not visible to qcom_smem_get_global().
> 
> Signed-off-by: Tao Zhang <quic_taozhan@...cinc.com>
> ---

Reviewed-by: Chris Lew <quic_clew@...cinc.com>