linux-kernel - Re: [PATCH] scsi: ufs: core: Ensure clk_gating.lock is used only after initialization

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: <20250122170702.jzwd57tk5dl7epwz@thinkpad>
Date: Wed, 22 Jan 2025 22:37:02 +0530
From: Manivannan Sadhasivam <manisadhasivam.linux@...il.com>
To: Avri Altman <Avri.Altman@....com>
Cc: "Martin K . Petersen" <martin.petersen@...cle.com>,
	"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Bart Van Assche <bvanassche@....org>,
	Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Subject: Re: [PATCH] scsi: ufs: core: Ensure clk_gating.lock is used only
 after initialization

On Wed, Jan 22, 2025 at 11:04:12AM +0000, Avri Altman wrote:
> > > To fix this issue, we use the existing `is_initialized` flag in the
> > > `clk_gating` structure to ensure that the spinlock is only used after
> > > it has been properly initialized. We check this flag before using the
> > > spinlock in the `ufshcd_setup_clocks` function.
> > >
> > > It was incorrect in the first place to call `setup_clocks()` before
> > > `ufshcd_init_clk_gating()`, and the introduction of the new lock
> > > unmasked this bug.
> > 
> > If calling setup_clocks() before ufshcd_init_clk_gating() is incorrect, why are
> > you not reordering it?
> > 
> > Checking for 'clk_gating.is_initialized' looks like a hack.
> Actually 'clk_gating.is_initialized' seems like the standard way to do this - see e.g. in hold and release.
> As for moving setup_clocks() around, I have some concerns about moving it out of ufshcd_hba_init.
> Having considered the alternatives, it seems that using 'clk_gating.is_initialized' ,
> despite its limitations, is the most practical solution we have.
> 
> I am open though for other suggestions.
> 

Looking at the code again, I think it is OK to have this fix for now. But
someone should spend some time to revisit the locking part in this driver.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>

- Mani

-- 
மணிவண்ணன் சதாசிவம்