netdev - Re: skb_shinfo(skb)->nr_frags > 0 while skb_is

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:	Tue, 6 Oct 2009 18:37:11 -0700
From:	"Michael Chan" <mchan@...adcom.com>
To:	"John Wright" <john.wright@...com>
cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"Bob Montgomery" <bob.montgomery@...com>
Subject: Re: skb_shinfo(skb)->nr_frags > 0 while skb_is_gso(skb) == 0?

On Tue, 2009-10-06 at 18:03 -0700, John Wright wrote:
> So, first, a question for someone who knows more about sk_buff's than
> I:
> is it reasonable/legal for an skb for which skb_is_gso(skb) == 0 to
> also
> have skb_shinfo(skb)->nr_frags > 0?
> 

As Stephen pointed out, yes.

> If yes, then for Michael, or someone familiar with bnx2 hardware: are
> "partial BD completions" (where the hw_tx_cons value is on a ring
> index
> that, on the tx_buf_ring, would have a NULL skb value, and on the
> tx_desc_ring, the tx_bd would not have the TX_BD_FLAGS_START flag set)
> possible only for gso skb's, or is it possible any time nr_frags > 0?
> 

Partial BD completions are only possible on TSO/GSO packets, unless
there is a hardware bug that we haven't found during all these years.

Several years ago, the same crash in bnx2 was found to be caused by HTB
corrupting nr_frags while the skb was queued by the driver.  That issue
has been fixed.  I wonder if skb->gso_size can change on us while the
skb is queued or we still have another case of changing nr_frags.

I think it will be good if you can run the same test on 2.6.31 where
is_gso and nr_frags are cached.  If the chip really does partial BD
completions on TX, you should still see the same issue whether we cache
these values or not.  If it doesn't crash on 2.6.31, then it may be
something else.

Thanks for the detailed debugging information.

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