netdev - Re: [PATCH]: cpmac bugfixes and enhancements

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, 5 May 2008 16:16:34 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Matteo Croce <matteo@...nwrt.org>
Cc:	jgarzik@...ox.com, ralf@...ux-mips.org, nbd@...nwrt.org,
	ejka@...i.kspu.ru, linux-mips@...ux-mips.org,
	netdev@...r.kernel.org
Subject: Re: [PATCH]: cpmac bugfixes and enhancements

On Sun, 4 May 2008 19:04:22 +0200
Matteo Croce <matteo@...nwrt.org> wrote:

> This patch fixes an IRQ storm, a locking issues, moves platform code in the right sections
> and other small fixes.
> 

Please feed this patch (and all future ones) through scripts/checkpatch.pl.
It picks up rather a lot of simple problems which there is no reason for
us to retain.

>
> ...
>
> +	spin_unlock(&priv->rx_lock);
> +	netif_rx_complete(priv->dev, napi);
> +	netif_stop_queue(priv->dev);
> +	napi_disable(&priv->napi);
> +	
> +	atomic_inc(&priv->reset_pending);
> +	cpmac_hw_stop(priv->dev);
> +	if (!schedule_work(&priv->reset_work))
> +		atomic_dec(&priv->reset_pending);
> +	return 0;
> + 
>  }
>  
>  static int cpmac_start_xmit(struct sk_buff *skb, struct net_device *dev)
> @@ -456,6 +549,9 @@ static int cpmac_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	struct cpmac_desc *desc;
>  	struct cpmac_priv *priv = netdev_priv(dev);
>  
> +	if (unlikely(atomic_read(&priv->reset_pending)))
> +		return NETDEV_TX_BUSY;
> +

This looks a bit strange.  schedule_work() will return zero if the work was
already scheduled, in which case we arrange for cpmac_start_xmit() to abort
early.

But if schedule_work() *doesn't* return zero, there is a time window in
which the reset is still pending.  Because it takes time for keventd to be
awoken and to run the work function.

I would have thought that we would want to prevent cpmac_start_xmit() from
running within that time window also?


But that's just a guess - the text which you used to describe your work is
missing much information, so I don't have a lot to work with here.
--
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