diff mbox series

scsi: ufs: ufshcd-pltfrm: Signedness bug in ufshcd_parse_clock_info()

Message ID 404a4727-89c6-410b-9ece-301fa399d4db@stanley.mountain (mailing list archive)
State Accepted
Headers show
Series scsi: ufs: ufshcd-pltfrm: Signedness bug in ufshcd_parse_clock_info() | expand

Commit Message

Dan Carpenter Aug. 15, 2024, 11:24 a.m. UTC
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(-)

Comments

Bart Van Assche Aug. 15, 2024, 5:47 p.m. UTC | #1
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.
Dan Carpenter Aug. 15, 2024, 9:35 p.m. UTC | #2
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
Manivannan Sadhasivam Aug. 16, 2024, 6:34 a.m. UTC | #3
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
>
Dan Carpenter Aug. 16, 2024, 11:46 a.m. UTC | #4
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
Martin K. Petersen Aug. 17, 2024, 1:07 a.m. UTC | #5
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 mbox series

Patch

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;