netdev - Re: [net-next 01/13] ixgb: eliminate checkstack warnings

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:	Mon, 19 Sep 2011 14:36:29 -0700
From:	Jesse Brandeburg <jesse.brandeburg@...el.com>
To:	Joe Perches <joe@...ches.com>
Cc:	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"gospo@...hat.com" <gospo@...hat.com>
Subject: Re: [net-next 01/13] ixgb: eliminate checkstack warnings

On Sat, 17 Sep 2011 01:43:26 -0700
Joe Perches <joe@...ches.com> wrote:

> On Sat, 2011-09-17 at 01:04 -0700, Jeff Kirsher wrote:
> > From: Jesse Brandeburg <jesse.brandeburg@...el.com>
> > Really trivial fix, use kzalloc/kree instead of stack space.
> 
> Some more trivialities...
> 
> > diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> > b/drivers/net/ethernet/intel/ixgb/ixgb_main.c\
> []
> > @@ -1120,8 +1120,12 @@ ixgb_set_multi(struct net_device *netdev)
> >  		rctl |= IXGB_RCTL_MPE;
> >  		IXGB_WRITE_REG(hw, RCTL, rctl);
> >  	} else {
> > -		u8 mta[IXGB_MAX_NUM_MULTICAST_ADDRESSES *
> > -			    IXGB_ETH_LENGTH_OF_ADDRESS];
> > +		u8 *mta = kzalloc(IXGB_MAX_NUM_MULTICAST_ADDRESSES
> > *
> > +			      IXGB_ETH_LENGTH_OF_ADDRESS,
> > GFP_KERNEL);
> 
> This doesn't need to be kzalloc as every byte is overwritten.
> It should be kmalloc.

done, V2 on its way

> Maybe delete the #define IXGB_ETH_LENGTH_OF_ADDRESS and
> sed 's/\bIXGB_ETH_LENGTH_OF_ADDRESS\b/ETH_ALEN/g' ?

done

 
> Perhaps this loop could be clearer without the multiply:
> 
> 		i = 0;
> 		netdev_for_each_mc_addr(ha, netdev)
> 			memcpy(&mta[i++ * IXGB_ETH_LENGTH_OF_ADDRESS],
> 			       ha->addr, IXGB_ETH_LENGTH_OF_ADDRESS);
> 
> Perhaps:
> 
> 		u8 *addr = mta;
> 		netdev_for_each_mc_addr(ha, netdev) {
> 			memcpy(addr, ha->addr, ETH_ALEN);
> 			addr += ETH_ALEN;
> 		}

done, but because of the nature of the changes being code flow, I'm
going to retest through our lab.  V2 will hopefully be at the list
shortly.

Thanks for the feedback,
 Jesse
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html