netdev - RE: [PATCH] stmmac: Clear variable when destroying workqueue

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: <000001da68a3$cc05a190$6410e4b0$@samsung.com>
Date: Mon, 26 Feb 2024 12:06:02 +0100
From: "Jakub Raczynski" <j.raczynski@...sung.com>
To: "'Jakub Kicinski'" <kuba@...nel.org>
Cc: <netdev@...r.kernel.org>, <alexandre.torgue@...s.st.com>,
	<joabreu@...opsys.com>
Subject: RE: [PATCH] stmmac: Clear variable when destroying workqueue


On Wed, 21 Feb 2024 15:32:33 +0100 Jakub Raczynski wrote:
>> Currently when suspending driver and stopping workqueue it is checked 
>> whether workqueue is not NULL and if so, it is destroyed.
>> Function destroy_workqueue() does drain queue and does clear variable, 
>> but it does not set workqueue variable to NULL. This can cause 
>> kernel/module panic if code attempts to clear workqueue that was not
initialized.

> Adding __FPE_REMOVING for allocation, it would be something, but failure
here is less that likely. DMA engine start can happen since some Synopsys IP
have specific clock timing requirements for
> DMA, which sometimes must be provided by another driver (if for example
PHY is driven by GPIO or PHY uses low-power mode during suspend).
> As for queued work, you are right, additional check for __FPE_REMOVING and
NULL check should be added to stmmac_service_event_schedule(), as is in
stmmac_fpe_event_status().
> Will re-test that and resend patch as requested.

Scratch that, confused main workqueue with fpe_workqueue in that message.
Proposed commit should not introduce problem with fpe_workqueue, since in
stmmac_fpe_event_status() there is check for both NULL and __FPE_REMOVING
before queueing work.
Will re-check if there are additional calls before submitting commit.

As for the addition of __FPE_REMOVING to initialization fail, would prefer
that in different commit.