netdev - RE: [PATCH -next] qlcnic: fix compiler warning

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]

Date:	Fri, 10 Jan 2014 10:15:54 +0000
From:	David Laight <David.Laight@...LAB.COM>
To:	'Shahed Shaikh' <shahed.shaikh@...gic.com>,
	Martin Kaiser <martin@...ser.cx>,
	Himanshu Madhani <himanshu.madhani@...gic.com>,
	"Rajesh Borundia" <rajesh.borundia@...gic.com>
CC:	linux-kernel <linux-kernel@...r.kernel.org>,
	"trivial@...nel.org" <trivial@...nel.org>,
	netdev <netdev@...r.kernel.org>
Subject: RE: [PATCH -next] qlcnic: fix compiler warning

From: Shahed Shaikh
> Adding netdev.
> 
> > -----Original Message-----
> > From: Martin Kaiser,,, [mailto:martin@...kholt.kaiser.cx] On Behalf Of
> > Martin Kaiser
> > Sent: Thursday, January 09, 2014 9:29 PM
> > To: Himanshu Madhani; Rajesh Borundia
> > Cc: linux-kernel; trivial@...nel.org
> > Subject: [PATCH -next] qlcnic: fix compiler warning
> >
> > Add an explicit cast to fix the following warning (seen on Debian Wheezy, gcc
> > 4.7.2)
> >
> > CC [M]  drivers/net/wireless/rtlwifi/rtl8192ce/trx.o
> >     drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c: In function
> > qlcnic_send_filter:
> >     drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c:349:3: warning:
> >     passing argument 2 of ether_addr_equal from incompatible pointer type
> > [enabled by default]
> >     In file included from include/linux/if_vlan.h:16:0,
> >     from drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c:9:
> >     include/linux/etherdevice.h:244:20: note: expected const u8 * but
> > argument is of type u64 *
> >
> 
> If I am not wrong, this patch should go to David's net-next tree.
> 
> > Signed-off-by: Martin Kaiser <martin@...ser.cx>
> 
> Acked-by: Shahed Shaikh <shahed.shaikh@...gic.com>
> 
> Thanks,
> Shahed
> > ---
> >  drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
> > b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
> > index 6373f60..3557154 100644
> > --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
> > +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
> > @@ -346,7 +346,7 @@ static void qlcnic_send_filter(struct qlcnic_adapter
> > *adapter,
> >  	head = &(adapter->fhash.fhead[hindex]);
> >
> >  	hlist_for_each_entry_safe(tmp_fil, n, head, fnode) {
> > -		if (ether_addr_equal(tmp_fil->faddr, &src_addr) &&
> > +		if (ether_addr_equal(tmp_fil->faddr, (const u8 *)&src_addr)
> > &&
> >  		    tmp_fil->vlan_id == vlan_id) {
> >  			if (jiffies > (QLCNIC_READD_AGE * HZ + tmp_fil-
> > >ftime))
> >  				qlcnic_change_filter(adapter, &src_addr,
> > --
> > 1.7.10.4

A quick look at the code seems to imply that this code is somewhat buggy.
'src_addr' is defined a u64 even though it is a 6 byte mac address.
On big-endian systems the wrong bytes get accessed all over the place.

Also when ether_addr_equal() in inlined the code ends up accessing src_addr
as 32bit and 16bit data - which don't have to be synchronised against
and writes to the 64bit item.

If might be that ether_addr_equal() (etc) should have a gcc asm
statement with a "memory" constraint against the mac address buffers.

	David