| Message ID | 1297375149-18458-2-git-send-email-greearb@candelatech.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
Le jeudi 10 février 2011 à 13:59 -0800, greearb@candelatech.com a écrit : > From: Ben Greear <greearb@candelatech.com> > > This allows user-space to send a '1500' MTU VLAN packet on a > 1500 MTU ethernet frame. The extra 4 bytes of a VLAN header is > not usually charged against the MTU when other parts of the > network stack is transmitting vlans... > > Signed-off-by: Ben Greear <greearb@candelatech.com> > --- > :100644 100644 91cb1d7... ef7f378... M net/packet/af_packet.c > net/packet/af_packet.c | 31 +++++++++++++++++++++++++++++-- > 1 files changed, 29 insertions(+), 2 deletions(-) > > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > index 91cb1d7..ef7f378 100644 > --- a/net/packet/af_packet.c > +++ b/net/packet/af_packet.c > @@ -466,7 +466,7 @@ retry: > */ > > err = -EMSGSIZE; > - if (len > dev->mtu + dev->hard_header_len) > + if (len > dev->mtu + dev->hard_header_len + VLAN_HLEN) > goto out_unlock; > > if (!skb) { > @@ -497,6 +497,19 @@ retry: > goto retry; > } > > + if (len > (dev->mtu + dev->hard_header_len)) { > + /* Earlier code assumed this would be a VLAN pkt, > + * double-check this now that we have the actual > + * packet in hand. > + */ > + struct ethhdr *ehdr; > + skb_reset_mac_header(skb); > + ehdr = eth_hdr(skb); > + if (ehdr->h_proto != htons(ETH_P_8021Q)) { > + err = -EMSGSIZE; > + goto out_unlock; This would leak skb. > + } > + } > > skb->protocol = proto; > skb->dev = dev; -- 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 02/10/2011 10:57 PM, Eric Dumazet wrote: > Le jeudi 10 février 2011 à 13:59 -0800, greearb@candelatech.com a > écrit : >> From: Ben Greear<greearb@candelatech.com> >> >> This allows user-space to send a '1500' MTU VLAN packet on a >> 1500 MTU ethernet frame. The extra 4 bytes of a VLAN header is >> not usually charged against the MTU when other parts of the >> network stack is transmitting vlans... >> >> Signed-off-by: Ben Greear<greearb@candelatech.com> >> --- >> :100644 100644 91cb1d7... ef7f378... M net/packet/af_packet.c >> net/packet/af_packet.c | 31 +++++++++++++++++++++++++++++-- >> 1 files changed, 29 insertions(+), 2 deletions(-) >> >> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c >> index 91cb1d7..ef7f378 100644 >> --- a/net/packet/af_packet.c >> +++ b/net/packet/af_packet.c >> @@ -466,7 +466,7 @@ retry: >> */ >> >> err = -EMSGSIZE; >> - if (len> dev->mtu + dev->hard_header_len) >> + if (len> dev->mtu + dev->hard_header_len + VLAN_HLEN) >> goto out_unlock; >> >> if (!skb) { >> @@ -497,6 +497,19 @@ retry: >> goto retry; >> } >> >> + if (len> (dev->mtu + dev->hard_header_len)) { >> + /* Earlier code assumed this would be a VLAN pkt, >> + * double-check this now that we have the actual >> + * packet in hand. >> + */ >> + struct ethhdr *ehdr; >> + skb_reset_mac_header(skb); >> + ehdr = eth_hdr(skb); >> + if (ehdr->h_proto != htons(ETH_P_8021Q)) { >> + err = -EMSGSIZE; >> + goto out_unlock; > > This would leak skb. > >> + } >> + } >> >> skb->protocol = proto; >> skb->dev = dev; Can you double-check that? Seems to me that in that method the out_unlock falls through to the out_free case. The other method is the opposite, funny enough... Thanks, Ben > > -- > 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
Le vendredi 11 février 2011 à 09:38 -0800, Ben Greear a écrit : > Can you double-check that? Seems to me that in that method the out_unlock > falls through to the out_free case. The other method is the opposite, funny > enough... Oops you're right, sorry. Reading again your patch, I see you used one ETH_HLEN instead of VLAN_HLEN in packet_snd() -- 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
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 91cb1d7..ef7f378 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -466,7 +466,7 @@ retry: */ err = -EMSGSIZE; - if (len > dev->mtu + dev->hard_header_len) + if (len > dev->mtu + dev->hard_header_len + VLAN_HLEN) goto out_unlock; if (!skb) { @@ -497,6 +497,19 @@ retry: goto retry; } + if (len > (dev->mtu + dev->hard_header_len)) { + /* Earlier code assumed this would be a VLAN pkt, + * double-check this now that we have the actual + * packet in hand. + */ + struct ethhdr *ehdr; + skb_reset_mac_header(skb); + ehdr = eth_hdr(skb); + if (ehdr->h_proto != htons(ETH_P_8021Q)) { + err = -EMSGSIZE; + goto out_unlock; + } + } skb->protocol = proto; skb->dev = dev; @@ -1200,7 +1213,7 @@ static int packet_snd(struct socket *sock, } err = -EMSGSIZE; - if (!gso_type && (len > dev->mtu+reserve)) + if (!gso_type && (len > dev->mtu + reserve + ETH_HLEN)) goto out_unlock; err = -ENOBUFS; @@ -1225,6 +1238,20 @@ static int packet_snd(struct socket *sock, if (err < 0) goto out_free; + if (!gso_type && (len > dev->mtu + reserve)) { + /* Earlier code assumed this would be a VLAN pkt, + * double-check this now that we have the actual + * packet in hand. + */ + struct ethhdr *ehdr; + skb_reset_mac_header(skb); + ehdr = eth_hdr(skb); + if (ehdr->h_proto != htons(ETH_P_8021Q)) { + err = -EMSGSIZE; + goto out_free; + } + } + skb->protocol = proto; skb->dev = dev; skb->priority = sk->sk_priority;