| Message ID | 50DA6B0D.6010500@linux-ipv6.org |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, 2012-12-26 at 12:12 +0900, YOSHIFUJI Hideaki wrote: > pskb_may_pull(skb, len) ensures that len bytes from skb->data > are available in a linear array. When pskb_may_pull() is > being used multiple times for the same buffer without > skb_pull(), the length is not accumulated. > > For example, assuming that we have done: > pskb_may_pull(skb, sizeof(struct icmp6hdr)) > > Here, we have to do: > pskb_may_pull(skb, sizeof(struct mld2_query)) > instead of: > pskb_may_pull(skb, sizeof(struct mld2_query) - > sizeof(struct icmp6hdr)) > > Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org> > --- > net/ipv6/mcast.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c > index 28dfa5f..5d91832 100644 > --- a/net/ipv6/mcast.c > +++ b/net/ipv6/mcast.c > @@ -1124,7 +1124,7 @@ int igmp6_event_query(struct sk_buff *skb) > int mark = 0; > int len; > > - if (!pskb_may_pull(skb, sizeof(struct in6_addr))) > + if (!pskb_may_pull(skb, sizeof(struct icmp6hdr) + sizeof(struct in6_addr))) > return -EINVAL; > I am a bit confused by your patch. igmp6_event_query() is called from icmpv6_rcv() _after_ pskb_pull(skb, sizeof(*hdr); (hdr being struct icmp6hdr) So this patch is wrong IMHO -- 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
Eric Dumazet wrote: > On Wed, 2012-12-26 at 12:12 +0900, YOSHIFUJI Hideaki wrote: >> pskb_may_pull(skb, len) ensures that len bytes from skb->data >> are available in a linear array. When pskb_may_pull() is >> being used multiple times for the same buffer without >> skb_pull(), the length is not accumulated. : >> @@ -1124,7 +1124,7 @@ int igmp6_event_query(struct sk_buff *skb) >> int mark = 0; >> int len; >> >> - if (!pskb_may_pull(skb, sizeof(struct in6_addr))) >> + if (!pskb_may_pull(skb, sizeof(struct icmp6hdr) + sizeof(struct in6_addr))) >> return -EINVAL; >> > > I am a bit confused by your patch. > > igmp6_event_query() is called from icmpv6_rcv() _after_ > > pskb_pull(skb, sizeof(*hdr); > > (hdr being struct icmp6hdr) > > So this patch is wrong IMHO Argh..I agree. I withdraw this one. --yoshfuji -- 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/ipv6/mcast.c b/net/ipv6/mcast.c index 28dfa5f..5d91832 100644 --- a/net/ipv6/mcast.c +++ b/net/ipv6/mcast.c @@ -1124,7 +1124,7 @@ int igmp6_event_query(struct sk_buff *skb) int mark = 0; int len; - if (!pskb_may_pull(skb, sizeof(struct in6_addr))) + if (!pskb_may_pull(skb, sizeof(struct icmp6hdr) + sizeof(struct in6_addr))) return -EINVAL; /* compute payload length excluding extension headers */ @@ -1165,9 +1165,7 @@ int igmp6_event_query(struct sk_buff *skb) /* clear deleted report items */ mld_clear_delrec(idev); } else if (len >= 28) { - int srcs_offset = sizeof(struct mld2_query) - - sizeof(struct icmp6hdr); - if (!pskb_may_pull(skb, srcs_offset)) + if (!pskb_may_pull(skb, sizeof(struct mld2_query))) return -EINVAL; mlh2 = (struct mld2_query *)skb_transport_header(skb); @@ -1186,8 +1184,9 @@ int igmp6_event_query(struct sk_buff *skb) } /* mark sources to include, if group & source-specific */ if (mlh2->mld2q_nsrcs != 0) { - if (!pskb_may_pull(skb, srcs_offset + - ntohs(mlh2->mld2q_nsrcs) * sizeof(struct in6_addr))) + if (!pskb_may_pull(skb, + sizeof(struct mld2_query) + + ntohs(mlh2->mld2q_nsrcs) * sizeof(struct in6_addr))) return -EINVAL; mlh2 = (struct mld2_query *)skb_transport_header(skb); @@ -1248,7 +1247,7 @@ int igmp6_event_report(struct sk_buff *skb) skb->pkt_type != PACKET_BROADCAST) return 0; - if (!pskb_may_pull(skb, sizeof(*mld) - sizeof(struct icmp6hdr))) + if (!pskb_may_pull(skb, sizeof(*mld))) return -EINVAL; mld = (struct mld_msg *)icmp6_hdr(skb);
pskb_may_pull(skb, len) ensures that len bytes from skb->data are available in a linear array. When pskb_may_pull() is being used multiple times for the same buffer without skb_pull(), the length is not accumulated. For example, assuming that we have done: pskb_may_pull(skb, sizeof(struct icmp6hdr)) Here, we have to do: pskb_may_pull(skb, sizeof(struct mld2_query)) instead of: pskb_may_pull(skb, sizeof(struct mld2_query) - sizeof(struct icmp6hdr)) Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org> --- net/ipv6/mcast.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)