linux-kernel - Re: [PATCH v3 2/3] scsi: ufs: core: Cleanup ufshcd_slave

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]

Date:   Sat, 22 Oct 2022 23:30:27 +0200
From:   Bean Huo <huobean@...il.com>
To:     Bart Van Assche <bvanassche@....org>, alim.akhtar@...sung.com,
        avri.altman@....com, asutoshd@...eaurora.org, jejb@...ux.ibm.com,
        martin.petersen@...cle.com, stanley.chu@...iatek.com,
        beanhuo@...ron.com, tomas.winkler@...el.com, cang@...eaurora.org,
        daejun7.park@...sung.com
Cc:     linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/3] scsi: ufs: core: Cleanup ufshcd_slave_alloc()

On Tue, 2022-10-18 at 11:27 -0700, Bart Van Assche wrote:
> Should a test be added that verifies that UNIT_DESC_PARAM_LU_Q_DEPTH
> < len?
> 
> Additionally, please use braces ({}) around multi-line if-statement
> bodies.
> 
> > +       /*
> > +        * According to UFS device specification, the write
> > protection mode is only supported by
> > +        * normal LU, not supported by WLUN.
> > +        */
> > +       if (hba->dev_info.f_power_on_wp_en && lun < hba-
> > >dev_info.max_lu_supported &&
> > +           !hba->dev_info.is_lu_power_on_wp &&
> > +           desc_buf[UNIT_DESC_PARAM_LU_WR_PROTECT] ==
> > UFS_LU_POWER_ON_WP)
> > +               hba->dev_info.is_lu_power_on_wp = true;
> 
> Also here, should the following test be added: 
> UNIT_DESC_PARAM_LU_WR_PROTECT < len?
> 
> Otherwise this patch looks good to me.

Bart, 

len is already the LU descriptor size. UNIT_DESC_PARAM_LU_Q_DEPTH (0x6)
and UNIT_DESC_PARAM_LU_WR_PROTECT(0x5) are very basic parameters in the
LU descriptor, no matter what version of UFS is, there are these two
parameters in the descriptor. So the return value of
ufshcd_read_unit_desc_param() will be -EOPNOTSUPP, -EINVAL, -ENOMEM or
0 (successful read of LU descriptor). the checkup of "< len" is not
neccessary.

Kind regards,
Bean