netdev - RE: [PATCH net-next 6/7] drivers: net: smc911x: Fix cast from pointer to integer of different size

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:   Tue, 3 Nov 2020 10:16:46 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Jakub Kicinski' <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>
CC:     netdev <netdev@...r.kernel.org>, Nicolas Pitre <nico@...xnic.net>
Subject: RE: [PATCH net-next 6/7] drivers: net: smc911x: Fix cast from pointer
 to integer of different size

From: Jakub Kicinski
> Sent: 02 November 2020 23:48
> 
> On Sat, 31 Oct 2020 01:49:57 +0100 Andrew Lunn wrote:
> > drivers/net/ethernet/smsc/smc911x.c: In function ‘smc911x_hardware_send_pkt’:
> > drivers/net/ethernet/smsc/smc911x.c:471:11: warning: cast from pointer to integer of different size
> [-Wpointer-to-int-cast]
> >   471 |  cmdA = (((u32)skb->data & 0x3) << 16) |
> >
> > When built on 64bit targets, the skb->data pointer cannot be cast to a
> > u32 in a meaningful way. Use long instead.
> >
> > Signed-off-by: Andrew Lunn <andrew@...n.ch>
> > ---
> >  drivers/net/ethernet/smsc/smc911x.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/smsc/smc911x.c b/drivers/net/ethernet/smsc/smc911x.c
> > index 4ec292563f38..f37832540364 100644
> > --- a/drivers/net/ethernet/smsc/smc911x.c
> > +++ b/drivers/net/ethernet/smsc/smc911x.c
> > @@ -466,9 +466,9 @@ static void smc911x_hardware_send_pkt(struct net_device *dev)
> >  			TX_CMD_A_INT_FIRST_SEG_ | TX_CMD_A_INT_LAST_SEG_ |
> >  			skb->len;
> >  #else
> > -	buf = (char*)((u32)skb->data & ~0x3);
> > -	len = (skb->len + 3 + ((u32)skb->data & 3)) & ~0x3;
> > -	cmdA = (((u32)skb->data & 0x3) << 16) |
> > +	buf = (char *)((long)skb->data & ~0x3);
> > +	len = (skb->len + 3 + ((long)skb->data & 3)) & ~0x3;
> > +	cmdA = (((long)skb->data & 0x3) << 16) |
> 
> Probably best if you swap the (long) for something unsigned here as
> well.

It would be much clearer with a temporary variable:
	offset = (unsigned long)skb->data & 3;
	buf = skb->data - offset;
	len = skb->len + offset;
	cmdA = offset << 16 | ...

   David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)