linux-kernel - Re: [PATCH v4 2/2] scsi: ufs: Protect PM ops and err

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] [day] [month] [year] [list]

Message-ID: <6908e7103529d12fd6ca0e5fa696b4bc@codeaurora.org>
Date:   Thu, 14 Jan 2021 08:48:06 +0800
From:   Can Guo <cang@...eaurora.org>
To:     Adrian Hunter <adrian.hunter@...el.com>
Cc:     asutoshd@...eaurora.org, nguyenb@...eaurora.org,
        hongwus@...eaurora.org, rnayak@...eaurora.org,
        linux-scsi@...r.kernel.org, kernel-team@...roid.com,
        saravanak@...gle.com, salyzyn@...gle.com,
        Alim Akhtar <alim.akhtar@...sung.com>,
        Avri Altman <avri.altman@....com>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Stanley Chu <stanley.chu@...iatek.com>,
        Bean Huo <beanhuo@...ron.com>,
        Nitin Rawat <nitirawa@...eaurora.org>,
        Satya Tangirala <satyat@...gle.com>,
        open list <linux-kernel@...r.kernel.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-arm-kernel@...ts.infradead.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@...ts.infradead.org>
Subject: Re: [PATCH v4 2/2] scsi: ufs: Protect PM ops and err_handler from
 user access through sysfs

On 2021-01-13 22:53, Adrian Hunter wrote:
> On 13/01/21 3:57 pm, Can Guo wrote:
>> User layer may access sysfs nodes when system PM ops or error handling
>> is running, which can cause various problems. Rename eh_sem to 
>> host_sem
>> and use it to protect PM ops and error handling from user layer 
>> intervene.
>> 
>> Acked-by: Avri Altman <avri.altman@....com>
>> Reviewed-by: Stanley Chu <stanley.chu@...iatek.com>
>> Signed-off-by: Can Guo <cang@...eaurora.org>
>> ---
>>  drivers/scsi/ufs/ufs-sysfs.c | 106 
>> ++++++++++++++++++++++++++++++++++++-------
>>  drivers/scsi/ufs/ufshcd.c    |  42 ++++++++++-------
>>  drivers/scsi/ufs/ufshcd.h    |  10 +++-
>>  3 files changed, 125 insertions(+), 33 deletions(-)
>> 
>> diff --git a/drivers/scsi/ufs/ufs-sysfs.c 
>> b/drivers/scsi/ufs/ufs-sysfs.c
>> index 0e14384..7cafffc 100644
>> --- a/drivers/scsi/ufs/ufs-sysfs.c
>> +++ b/drivers/scsi/ufs/ufs-sysfs.c
>> @@ -154,18 +154,29 @@ static ssize_t auto_hibern8_show(struct device 
>> *dev,
>>  				 struct device_attribute *attr, char *buf)
>>  {
>>  	u32 ahit;
>> +	int ret;
>>  	struct ufs_hba *hba = dev_get_drvdata(dev);
>> 
>>  	if (!ufshcd_is_auto_hibern8_supported(hba))
>>  		return -EOPNOTSUPP;
>> 
>> +	down(&hba->host_sem);
>> +	if (!ufshcd_is_sysfs_allowed(hba)) {
> 
> I expect debugfs has the same potential problem, so maybe
> ufshcd_is_sysfs_allowed() is not quite the right name.

I noticed your debugfs change - currently it is only printing
error counts recorded in hba struct, which is fine.
Even in this patch, the check is only added for those entries
which need to talk with HW. Sysfs nodes like show/store rpm/spm_lvl
does not need this check.

Thanks,
Can Guo.

> 
>> +		ret = -EBUSY;
>> +		goto out;
>> +	}
>> +