netdev - Re: [PATCH] netpoll: fix race on poll_list resulting in garbage entry

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20081218090414.GB8416@ff.dom.local>
Date:	Thu, 18 Dec 2008 09:04:14 +0000
From:	Jarek Poplawski <jarkao2@...il.com>
To:	Neil Horman <nhorman@...driver.com>
Cc:	Stephen Hemminger <shemminger@...tta.com>,
	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH] netpoll: fix race on poll_list resulting in garbage
	entry

On Wed, Dec 17, 2008 at 08:13:06PM -0500, Neil Horman wrote:
...
> Since we migrated the napi polling infrastructure out of the net_device
> structure, the netif_rx_[prep|schedule|complete] api has taken a net_device
> structure pointer, which in all cases goes unused.  This patch modifies the api
> to remove that parameter, and fixes up all the required call sites.
> 
> I've obviously not tested it with all available NICS, but I built an
> allmodconfig sucessfully with no errors introduced, and booted a kernel with
> htis change on a few systems.
> 
> Regards
> Neil
> 
> Signed-off-by: Neil Horman <nhorman@...driver.com>
...
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index e26f549..d2f692d 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1444,8 +1444,7 @@ static inline u32 netif_msg_init(int debug_value, int default_msg_enable_bits)
>  }
>  
>  /* Test if receive needs to be scheduled but only if up */
> -static inline int netif_rx_schedule_prep(struct net_device *dev,
> -					 struct napi_struct *napi)
> +static inline int netif_rx_schedule_prep(struct napi_struct *napi)
>  {
>  	return napi_schedule_prep(napi);
>  }
> @@ -1453,27 +1452,24 @@ static inline int netif_rx_schedule_prep(struct net_device *dev,
>  /* Add interface to tail of rx poll list. This assumes that _prep has
>   * already been called and returned 1.
>   */
> -static inline void __netif_rx_schedule(struct net_device *dev,
> -				       struct napi_struct *napi)
> +static inline void __netif_rx_schedule(struct napi_struct *napi)
>  {
>  	__napi_schedule(napi);
>  }

My proposal is to remove this duplication (in another patch/patches) by
renaming __napi_schedule() to __netif_rx_schedule() etc. (__netif_rx
instead of __napi as much as possible).

Jarek P.

>  
>  /* Try to reschedule poll. Called by irq handler. */
>  
> -static inline void netif_rx_schedule(struct net_device *dev,
> -				     struct napi_struct *napi)
> +static inline void netif_rx_schedule(struct napi_struct *napi)
>  {
> -	if (netif_rx_schedule_prep(dev, napi))
> -		__netif_rx_schedule(dev, napi);
> +	if (netif_rx_schedule_prep(napi))
> +		__netif_rx_schedule(napi);
>  }
>  
>  /* Try to reschedule poll. Called by dev->poll() after netif_rx_complete().  */
> -static inline int netif_rx_reschedule(struct net_device *dev,
> -				      struct napi_struct *napi)
> +static inline int netif_rx_reschedule(struct napi_struct *napi)
>  {
>  	if (napi_schedule_prep(napi)) {
> -		__netif_rx_schedule(dev, napi);
> +		__netif_rx_schedule(napi);
>  		return 1;
>  	}
>  	return 0;
> @@ -1482,8 +1478,7 @@ static inline int netif_rx_reschedule(struct net_device *dev,
>  /* same as netif_rx_complete, except that local_irq_save(flags)
>   * has already been issued
>   */
> -static inline void __netif_rx_complete(struct net_device *dev,
> -				       struct napi_struct *napi)
> +static inline void __netif_rx_complete(struct napi_struct *napi)
>  {
>  	__napi_complete(napi);
>  }
> @@ -1493,8 +1488,7 @@ static inline void __netif_rx_complete(struct net_device *dev,
>   * it completes the work. The device cannot be out of poll list at this
>   * moment, it is BUG().
>   */
> -static inline void netif_rx_complete(struct net_device *dev,
> -				     struct napi_struct *napi)
> +static inline void netif_rx_complete(struct napi_struct *napi)
>  {
>  	unsigned long flags;
>  
> @@ -1505,7 +1499,7 @@ static inline void netif_rx_complete(struct net_device *dev,
>  	if (unlikely(test_bit(NAPI_STATE_NPSVC, &napi->state)))
>  		return;
>  	local_irq_save(flags);
> -	__netif_rx_complete(dev, napi);
> +	__netif_rx_complete(napi);
>  	local_irq_restore(flags);
>  }
>  
> -- 
> /****************************************************
>  * Neil Horman <nhorman@...driver.com>
>  * Software Engineer, Red Hat
>  ****************************************************/
--
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