netdev - Re: [PATCH v3 5/7] via-velocity: Re-enable transmit scatter-gather support

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:	Wed, 25 Nov 2009 15:37:35 -0800 (PST)
From:	David Miller <davem@...emloft.net>
To:	simon.kagstrom@...insight.net
Cc:	netdev@...r.kernel.org, davej@...hat.com, shemminger@...tta.com,
	romieu@...zoreil.com
Subject: Re: [PATCH v3 5/7] via-velocity: Re-enable transmit scatter-gather
 support

From: Simon Kagstrom <simon.kagstrom@...insight.net>
Date: Wed, 25 Nov 2009 09:22:13 +0100

> The velocity hardware can handle up to 7 memory segments. This can be
> turned on and off via ethtool. The support was removed in commit
> 
>   83c98a8cd04dd0f848574370594886ba3bf56750
> 
> but is re-enabled and cleaned up here. It's off by default.
> 
> Signed-off-by: Simon Kagstrom <simon.kagstrom@...insight.net>
> ---
> ChangeLog:
> 
>   * (David Miller) return NETDEV_TX_OK from the velocity_xmit
>     function. I'm still a bit unsure on what to actually return in
>     these cases (they are error cases), but other drivers seem to
>     return NETDEV_TX_OK, so that's what I went with.

This still isn't right, sorry.

If you return NETDEV_TX_OK you must free the packet up, either
immediately or in response to the TX interrupt which finishes the
sending of the TX packet.

The case where you aren't getting this right:

+	/* If it's still above 6 we can't do anything */
+	if (skb_shinfo(skb)->nr_frags > 6) {
+		dev_err(&vptr->pdev->dev,
+				"via-velocity: more than 6 frags, can't send.\n");
+		return NETDEV_TX_OK;
+	}

is bogus because you just did __skb_linearize() and it returned zero, therefore
it would be illegal to see ->nr_frags with a > 6 value here.

Just remove this check and block of code entirely.
--
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