Re: [PATCH 2/2] scsi: ufs: Protect PM ops and err_handler from user access through sysfs

[Bean Huo]

On Tue, 2021-01-05 at 09:07 +0800, Can Guo wrote:
> On 2021-01-05 04:05, Bean Huo wrote:
> > On Sat, 2021-01-02 at 05:59 -0800, Can Guo wrote:
> > > + * @shutting_down: flag to check if shutdown has been invoked
> > 
> > I am not much sure if this flag is need, since once PM going in
> > shutdown path, what will be returnded by pm_runtime_get_sync()?
> > 
> > If pm_runtime_get_sync() will fail, just check its return.
> > 
> 
> That depends. During/after shutdown, for UFS's case only,
> pm_runtime_get_sync(hba->dev) will most likely return 0,
> because it is already RUNTIME_ACTIVE, pm_runtime_get_sync()
> will directly return 0... meaning you cannot count on it.
> 
> Check Stanley's change -
> https://lore.kernel.org/patchwork/patch/1341389/
> 
> Can Guo.

Can,

Thanks for pointing out that.

Based on my understanding, that patch is redundent. maybe I
misundestood Linux shutdown sequence.

I checked the shutdown flow:
 
1. Set the "system_state" variable
2. Disable usermod to ensure that no user from userspace can start a
request
3. device_shutdown()
So, userspace thread can not start a request to trigger runtime
resume(pm_runtime_get_sync) after step 2.

also,  no need to add that flag to checkup if shutdwon is running,
maybe it is better to check variable system_state:

if (system_state == SYSTEM_POWER_OFF || system_state == SYSTEM_HALT
|| system_state == SYSTEM_RESTART) 
	//shutdown start


I still hope Rafael or someone else can confirm that if
pm_runtime_get_sync() will really return ok in shutdown flow.


thanks,
Bean

> 
> > Hi Rafael
> > would you please help us confirm this?
> > 
> > thanks,
> > Bean