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]

Message-ID: <20091126083641.4f961ce5@marrow.netinsight.se>
Date:	Thu, 26 Nov 2009 08:36:41 +0100
From:	Simon Kagstrom <simon.kagstrom@...insight.net>
To:	David Miller <davem@...emloft.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

On Wed, 25 Nov 2009 15:37:35 -0800 (PST)
David Miller <davem@...emloft.net> wrote:

> 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.

Thanks for the explanation, I'll submit an updated patch. I put the
check there because I was a bit unsure about what __skb_linearize()
could do to the number of fragments. In particular, __pskb_pull_tail()
ends with

  pull_pages:
	eat = delta;
	k = 0;
	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
		if (skb_shinfo(skb)->frags[i].size <= eat) {
			put_page(skb_shinfo(skb)->frags[i].page);
			eat -= skb_shinfo(skb)->frags[i].size;
		} else {
			skb_shinfo(skb)->frags[k] = skb_shinfo(skb)->frags[i];
			if (eat) {
				skb_shinfo(skb)->frags[k].page_offset += eat;
				skb_shinfo(skb)->frags[k].size -= eat;
				eat = 0;
			}
			k++;
		}
	}
	skb_shinfo(skb)->nr_frags = k;

	skb->tail     += delta;
	skb->data_len -= delta;

	return skb_tail_pointer(skb);
  }

and after having crunched this in my head for a while, I couldn't
convince myself that nr_frags would always come out as less than 7 from
here. I don't claim to have done a thorough examination though.


Anyway, patch coming up.

// Simon
--
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