From mboxrd@z Thu Jan  1 00:00:00 1970
From: Eugeniu Rosca <erosca@de.adit-jv.com>
Date: Mon, 27 Aug 2018 22:24:30 +0200
Subject: [U-Boot] [PATCH v2 06/13] net: ravb: Fix signed shift overflow
In-Reply-To: <43dc652f-9315-6514-fe1c-b24d214421cd@gmail.com>
References: <20180826231332.2491-1-erosca@de.adit-jv.com>
 <20180826231332.2491-7-erosca@de.adit-jv.com>
 <43dc652f-9315-6514-fe1c-b24d214421cd@gmail.com>
Message-ID: <20180827202430.GE18631@vmlxhi-102.adit-jv.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Marek,

On Mon, Aug 27, 2018 at 01:22:54AM +0200, Marek Vasut wrote:
> On 08/27/2018 01:13 AM, Eugeniu Rosca wrote:
[...]
> >  
> >  #define RAVB_DESC_DT(n)			((n) << 28)
> 
> What about changing this instead, ((u32)(n) << 28) ?

This works too.

[...]

> >  
> > -	writel((mac[0] << 24) | (mac[1] << 16) | (mac[2] << 8) | mac[3],
> > -	       eth->iobase + RAVB_REG_MAHR);
> > +	writel(((u32)mac[0] << 24) | ((u32)mac[1] << 16) | ((u32)mac[2] << 8) |
> > +	       mac[3], eth->iobase + RAVB_REG_MAHR);
> 
> Not a big fan of the casts here, I wonder if there isn't some more
> elegant solution. If not, so be it.

Actually one cast is enough to fix the UB.
Let me know if below patch looks better to you.

diff --git a/drivers/net/ravb.c b/drivers/net/ravb.c
index 749562db960e..2190811c53bb 100644
--- a/drivers/net/ravb.c
+++ b/drivers/net/ravb.c
@@ -72,7 +72,7 @@
 #define RAVB_TX_QUEUE_OFFSET		0
 #define RAVB_RX_QUEUE_OFFSET		4
 
-#define RAVB_DESC_DT(n)			((n) << 28)
+#define RAVB_DESC_DT(n)			((u32)(n) << 28)
 #define RAVB_DESC_DT_FSINGLE		RAVB_DESC_DT(0x7)
 #define RAVB_DESC_DT_LINKFIX		RAVB_DESC_DT(0x9)
 #define RAVB_DESC_DT_EOS		RAVB_DESC_DT(0xa)
@@ -342,7 +342,7 @@ static int ravb_write_hwaddr(struct udevice *dev)
 	struct eth_pdata *pdata = dev_get_platdata(dev);
 	unsigned char *mac = pdata->enetaddr;
 
-	writel((mac[0] << 24) | (mac[1] << 16) | (mac[2] << 8) | mac[3],
+	writel(((u32)mac[0] << 24) | (mac[1] << 16) | (mac[2] << 8) | mac[3],
 	       eth->iobase + RAVB_REG_MAHR);
 
 	writel((mac[4] << 8) | mac[5], eth->iobase + RAVB_REG_MALR);


Thanks,
Eugeniu.