netdev - Re: [PATCH] wifi: wilc1000: Rework bus locking

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: <5e2a5056-78ac-4be0-83ca-4aa55f524535@denx.de>
Date: Wed, 23 Oct 2024 16:44:44 +0200
From: Marek Vasut <marex@...x.de>
To: Ajay.Kathat@...rochip.com, alexis.lothore@...tlin.com,
 linux-wireless@...r.kernel.org
Cc: davem@...emloft.net, adham.abozaeid@...rochip.com,
 claudiu.beznea@...on.dev, conor+dt@...nel.org, edumazet@...gle.com,
 kuba@...nel.org, kvalo@...nel.org, krzk+dt@...nel.org, pabeni@...hat.com,
 robh@...nel.org, devicetree@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH] wifi: wilc1000: Rework bus locking

On 10/23/24 9:17 AM, Ajay.Kathat@...rochip.com wrote:

Hello Ajay,

>> What I am trying to say is this:
>>
>> With current code, this can happen, which is not good, because transfers
>> from multiple threads can be interleaved and interfere with each other:
> 
> Did you observe any bus failure in your test setup with SDIO. Is there any
> configure to recreate it.

I am observing sporadic command and data CRC errors on STM32MP157F 
system with SDIO WILC3000.

> The SDIO transfer may appear to be split into into multiple transaction but
> these calls should not impact each other since most of them are updating the
> different registers except WILC_SDIO_FBR_CSA_REG register, which is used for
> CSA read/write. If needed, wilc_sdio_set_func0_csa_address() can be modified
> to club the 3x CMD52 and 1x CMD53 into a single transfer API.
> 
> In my opinion, If sdio_claim_host() is moved to acquire_bus() API then the
> SDIO bus will be claimed longer than required especially in
> wilc_wlan_handle_txq() API. Ideally, calling sdio_claim_host() call just
> before the transfer is enough but now the SDIO I/O bus would be continuously
> blocked for multiple independent SDIO transactions that is not necessary.

Why would that pose a problem ?

I am more concerned that ksdioirqd can insert a command transfer right 
in the middle of CMD52/CMD53 register read composite transfer, because 
while ksdioirqd does use proper sdio_claim/release_host, this driver 
does it per-SDIO-command instead of per the whole e.g. register read 
"transaction".

[...]