Message ID | 20110113064310.729751199@de.ibm.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
From: frank.blaschka@de.ibm.com Date: Thu, 13 Jan 2011 07:42:25 +0100 > --- a/drivers/s390/net/qeth_l3_main.c > +++ b/drivers/s390/net/qeth_l3_main.c > @@ -2998,7 +2998,9 @@ static inline void qeth_l3_hdr_csum(stru > */ > if (iph->protocol == IPPROTO_UDP) > hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_UDP; > - hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ; > + hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ | > + QETH_HDR_EXT_CSUM_HDR_REQ; > + iph->check = 0; > if (card->options.performance_stats) > card->perf_stats.tx_csum++; > } You may not change the packet header contents blindly like this. Otherwise unpredictable contents will be seen by tcpdump and any other code path which has a clone of this packet. Thus, you'll need to guard this change with something like: if (skb_header_cloned(skb) && pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) { dev_kfree_skb(skb); goto tx_fail; } Please fix this and resubmit your two patches. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 12, 2011 at 11:47:35PM -0800, David Miller wrote: > From: frank.blaschka@de.ibm.com > Date: Thu, 13 Jan 2011 07:42:25 +0100 > > > --- a/drivers/s390/net/qeth_l3_main.c > > +++ b/drivers/s390/net/qeth_l3_main.c > > @@ -2998,7 +2998,9 @@ static inline void qeth_l3_hdr_csum(stru > > */ > > if (iph->protocol == IPPROTO_UDP) > > hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_UDP; > > - hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ; > > + hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ | > > + QETH_HDR_EXT_CSUM_HDR_REQ; > > + iph->check = 0; > > if (card->options.performance_stats) > > card->perf_stats.tx_csum++; > > } > > You may not change the packet header contents blindly like this. > Otherwise unpredictable contents will be seen by tcpdump and any > other code path which has a clone of this packet. > > Thus, you'll need to guard this change with something like: > > if (skb_header_cloned(skb) && > pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) { > dev_kfree_skb(skb); > goto tx_fail; > } Yes I know. Because of the suboptimal l3 driver design :-) we already have a private copy of the skb at this place. Thx! > > Please fix this and resubmit your two patches. > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Frank Blaschka <frank.blaschka@de.ibm.com> Date: Thu, 13 Jan 2011 09:23:59 +0100 > On Wed, Jan 12, 2011 at 11:47:35PM -0800, David Miller wrote: >> From: frank.blaschka@de.ibm.com >> Date: Thu, 13 Jan 2011 07:42:25 +0100 >> >> > --- a/drivers/s390/net/qeth_l3_main.c >> > +++ b/drivers/s390/net/qeth_l3_main.c >> > @@ -2998,7 +2998,9 @@ static inline void qeth_l3_hdr_csum(stru >> > */ >> > if (iph->protocol == IPPROTO_UDP) >> > hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_UDP; >> > - hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ; >> > + hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ | >> > + QETH_HDR_EXT_CSUM_HDR_REQ; >> > + iph->check = 0; >> > if (card->options.performance_stats) >> > card->perf_stats.tx_csum++; >> > } >> >> You may not change the packet header contents blindly like this. >> Otherwise unpredictable contents will be seen by tcpdump and any >> other code path which has a clone of this packet. >> >> Thus, you'll need to guard this change with something like: >> >> if (skb_header_cloned(skb) && >> pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) { >> dev_kfree_skb(skb); >> goto tx_fail; >> } > Yes I know. Because of the suboptimal l3 driver design :-) we already have > a private copy of the skb at this place. Thx! I see, thanks for explaining. Both patches applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/drivers/s390/net/qeth_l3_main.c +++ b/drivers/s390/net/qeth_l3_main.c @@ -2998,7 +2998,9 @@ static inline void qeth_l3_hdr_csum(stru */ if (iph->protocol == IPPROTO_UDP) hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_UDP; - hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ; + hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_TRANSP_REQ | + QETH_HDR_EXT_CSUM_HDR_REQ; + iph->check = 0; if (card->options.performance_stats) card->perf_stats.tx_csum++; }