linux-kernel - Re: [PATCH v2 1/2] scsi: ufs: core: Remove unnecessary if statement

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: <d06a264c4544a23ccccd016c6797d889db526b64.camel@iokpp.de>
Date:   Fri, 14 Oct 2022 21:20:44 +0200
From:   Bean Huo <beanhuo@...pp.de>
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 v2 1/2] scsi: ufs: core: Remove unnecessary if statement

On Fri, 2022-10-14 at 11:37 -0700, Bart Van Assche wrote:
> > @@ -300,9 +300,6 @@ static inline bool
> > ufs_is_valid_unit_desc_lun(struct ufs_dev_info *dev_info,
> >                 pr_err("Max General LU supported by UFS isn't
> > initialized\n");
> >                 return false;
> >         }
> > -       /* WB is available only for the logical unit from 0 to 7 */
> > -       if (param_offset == UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS)
> > -               return lun < UFS_UPIU_MAX_WB_LUN_ID;
> >         return lun == UFS_UPIU_RPMB_WLUN || (lun < dev_info-
> > >max_lu_supported);
> >    }
> 
> Hi Bean,
> 
> I think the above patch reintroduces the stack overflow issue fixed
> by
> commit a2fca52ee640 ("scsi: ufs: WB is only available on LUN #0 to
> #7").
> 
> How about reverting commit a2fca52ee640 and fixing the stack overflow
> issue in another way than by modifying ufs_is_valid_unit_desc_lun()?
> 
> Thanks,
> 
> Bart

Hi Bart, 

I knew that fix, it was because the user tried to poll
dLUNumWriteBoosterBufferAllocUnits from RPMB LU, as you know, RPMB
doesn't support WB, but the root cause is that we don't separate normal
logical unit descriptors and RPMB unit descriptor when we create sysfs
group,


in ufshcd_driver_template {

...
.sdev_groups = ufshcd_driver_groups,

}



ufshcd_driver_groups {
...
&ufs_sysfs_unit_descriptor_group,
...
}


so all the logical units will have the unified unit descriptor sysfs
node. This is wrong.  

Another problem is that Boot and device LU don't have unit descriptors,
but we still create a unit descriptor sysfs node group for boot and
device LU.

I am working on the Advanced RPMB, and trying to seperate normal unit
descriptor and RPMB unit descriptor, will let you know if it is
possible. or if you know the solution, please let me know, thanks.


Kind regards,
Bean