| Message ID | 404a4727-89c6-410b-9ece-301fa399d4db@stanley.mountain |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: ufs: ufshcd-pltfrm: Signedness bug in ufshcd_parse_clock_info() | expand |
On 8/15/24 4:24 AM, Dan Carpenter wrote: > The "sz" variable needs to be a signed type for the error handling to > work as intended. What error handling are you referring to? I haven't found any code that assigns a negative value to 'sz' in ufshcd_parse_clock_info(). Did I perhaps overlook something? Thanks, Bart.
On Thu, Aug 15, 2024 at 10:47:30AM -0700, Bart Van Assche wrote: > On 8/15/24 4:24 AM, Dan Carpenter wrote: > > The "sz" variable needs to be a signed type for the error handling to > > work as intended. > > What error handling are you referring to? I haven't found any code that > assigns a negative value to 'sz' in ufshcd_parse_clock_info(). Did I > perhaps overlook something? > Rob's patch in linux-next. - if (!of_get_property(np, "freq-table-hz", &len)) { + sz = of_property_count_u32_elems(np, "freq-table-hz"); + if (sz <= 0) { dev_info(dev, "freq-table-hz property not specified\n"); goto out; regards, dan carpenter
On Fri, Aug 16, 2024 at 12:35:22AM +0300, Dan Carpenter wrote: > On Thu, Aug 15, 2024 at 10:47:30AM -0700, Bart Van Assche wrote: > > On 8/15/24 4:24 AM, Dan Carpenter wrote: > > > The "sz" variable needs to be a signed type for the error handling to > > > work as intended. > > > > What error handling are you referring to? I haven't found any code that > > assigns a negative value to 'sz' in ufshcd_parse_clock_info(). Did I > > perhaps overlook something? > > > > Rob's patch in linux-next. > It would've been helpful if you added 'next' in the patch subject prefix. Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> - Mani > - if (!of_get_property(np, "freq-table-hz", &len)) { > + sz = of_property_count_u32_elems(np, "freq-table-hz"); > + if (sz <= 0) { > dev_info(dev, "freq-table-hz property not specified\n"); > goto out; > > regards, > dan carpenter >
On Fri, Aug 16, 2024 at 12:04:04PM +0530, Manivannan Sadhasivam wrote: > On Fri, Aug 16, 2024 at 12:35:22AM +0300, Dan Carpenter wrote: > > On Thu, Aug 15, 2024 at 10:47:30AM -0700, Bart Van Assche wrote: > > > On 8/15/24 4:24 AM, Dan Carpenter wrote: > > > > The "sz" variable needs to be a signed type for the error handling to > > > > work as intended. > > > > > > What error handling are you referring to? I haven't found any code that > > > assigns a negative value to 'sz' in ufshcd_parse_clock_info(). Did I > > > perhaps overlook something? > > > > > > > Rob's patch in linux-next. > > > > It would've been helpful if you added 'next' in the patch subject prefix. > I guess that would helped in this case. But most of the time when I see this question it's because there are two different upstream maintainers modifying the same code... Anyway, sure, I can change my script to add "next" to the subject when the FIXES_COMMIT isn't in Linus's tree. if [ "$FIXES_COMMIT" != "" ] ; then if ! git merge-base --is-ancestor $FIXES_COMMIT origin/master ; then TREE=" next" fi fi regards, dan carpenter
Dan, > The "sz" variable needs to be a signed type for the error handling to > work as intended. Fortunately, there is some sanity checking on "sz" > on the next line, so negative values would be caught and it doesn't > really affect runtime. Applied to 6.12/scsi-staging, thanks!
diff --git a/drivers/ufs/host/ufshcd-pltfrm.c b/drivers/ufs/host/ufshcd-pltfrm.c index 0c9b303ccfa0..1f4f30d6cb42 100644 --- a/drivers/ufs/host/ufshcd-pltfrm.c +++ b/drivers/ufs/host/ufshcd-pltfrm.c @@ -31,7 +31,7 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba) const char *name; u32 *clkfreq = NULL; struct ufs_clk_info *clki; - size_t sz = 0; + ssize_t sz = 0; if (!np) goto out;
The "sz" variable needs to be a signed type for the error handling to work as intended. Fortunately, there is some sanity checking on "sz" on the next line, so negative values would be caught and it doesn't really affect runtime. Fixes: eab0dce11dd9 ("scsi: ufs: ufshcd-pltfrm: Use of_property_count_u32_elems() to get property length") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- drivers/ufs/host/ufshcd-pltfrm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)