netdev - RE: [PATCH net-next v2 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <babda61688af4f42b4a9e0fb41808272@AcuMS.aculab.com>
Date:   Fri, 6 Nov 2020 08:44:20 +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>,
        "Lee Jones" <lee.jones@...aro.org>
Subject: RE: [PATCH net-next v2 6/7] drivers: net: smc911x: Fix cast from
 pointer to integer of different size

From: Jakub Kicinski
> Sent: 05 November 2020 22:47
> 
> On Wed,  4 Nov 2020 16:48:57 +0100 Andrew Lunn wrote:
> > -	buf = (char*)((u32)skb->data & ~0x3);
> > -	len = (skb->len + 3 + ((u32)skb->data & 3)) & ~0x3;
> > -	cmdA = (((u32)skb->data & 0x3) << 16) |
> > +	offset = (unsigned long)skb->data & 3;
> > +	buf = skb->data - offset;
> > +	len = skb->len + offset;
> > +	cmdA = (offset << 16) |
> 
> The len calculation is wrong, you trusted people on the mailing list
> too much ;)

I misread what the comment-free convoluted code was doing....

Clearly it is rounding the base address down to a multiple of 4
and passing the offset in cmdA.
This is fine - but quite why the (I assume) hardware doesn't do
this itself and just document that it does a 32bit read is
another matter - the logic will be much the same and I doubt
anything modern is that pushed for space.

However rounding the length up to a multiple of 4 is buggy.
If this is an ethernet chipset it must (honest) be able to
send frames that don't end on a 4 byte boundary.
So rounding up the length is very dubious.

	David

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