2005-10-31 05:19:12

by Yan Zheng

Subject: [PATCH][MCAST]IPv6: Check packet size when process Multicast Address and Source Specific Query

Signed-off-by: Yan Zheng <[email protected]>

Index: net/ipv6/mcast.c
================================================================================
--- linux-2.6.14/net/ipv6/mcast.c 2005-10-30 23:09:33.000000000 +0800
+++ linux/net/ipv6/mcast.c 2005-10-31 13:13:10.000000000 +0800
@@ -1156,7 +1156,12 @@ int igmp6_event_query(struct sk_buff *sk
return 0;
}
/* mark sources to include, if group & source-specific */
- mark = mlh2->nsrcs != 0;
+ if (mlh2->nsrcs != 0) {
+ if (!pskb_may_pull(skb, mlh2->nsrcs * sizeof(struct in6_addr) +
+ (sizeof(struct mld2_query) - sizeof(struct icmp6hdr))))
+ return -EINVAL;
+ mark = 1;
+ }
} else {
in6_dev_put(idev);
return -EINVAL;

2005-10-31 05:27:15

by YOSHIFUJI Hideaki

Subject: Re: [PATCH][MCAST]IPv6: Check packet size when process Multicast Address and Source Specific Query

In article <[email protected]> (at Mon, 31 Oct 2005 13:20:21 +0800), Yan Zheng <[email protected]> says:

>
> Signed-off-by: Yan Zheng <[email protected]>
>
> Index: net/ipv6/mcast.c
> ================================================================================
> --- linux-2.6.14/net/ipv6/mcast.c 2005-10-30 23:09:33.000000000 +0800
> +++ linux/net/ipv6/mcast.c 2005-10-31 13:13:10.000000000 +0800
> @@ -1156,7 +1156,12 @@ int igmp6_event_query(struct sk_buff *sk
> return 0;
> }
> /* mark sources to include, if group & source-specific */
> - mark = mlh2->nsrcs != 0;
> + if (mlh2->nsrcs != 0) {
> + if (!pskb_may_pull(skb, mlh2->nsrcs * sizeof(struct in6_addr) +
> + (sizeof(struct mld2_query) - sizeof(struct icmp6hdr))))
> + return -EINVAL;
> + mark = 1;
> + }
> } else {
> in6_dev_put(idev);
> return -EINVAL;

You cannot continue using mlh2, local copy of skb->h.raw
after pskb_may_pull(). Please refresh it.

--yoshfuji

2005-10-31 12:08:49

by Yan Zheng

Subject: Re: [PATCH][MCAST]IPv6: Check packet size when process Multicast Address and Source Specific Query

>
> You cannot continue using mlh2, local copy of skb->h.raw
> after pskb_may_pull(). Please refresh it.
>
> --yoshfuji
>

My mistake. sorry.
I hope the new one is correct.

Regards
================================================================================
--- linux-2.6.14/net/ipv6/mcast.c 2005-10-30 23:09:33.000000000 +0800
+++ linux/net/ipv6/mcast.c 2005-10-31 14:16:19.000000000 +0800
@@ -1087,7 +1087,7 @@ static void mld_marksources(struct ifmca

int igmp6_event_query(struct sk_buff *skb)
{
- struct mld2_query *mlh2 = (struct mld2_query *) skb->h.raw;
+ struct mld2_query *mlh2 = NULL;
struct ifmcaddr6 *ma;
struct in6_addr *group;
unsigned long max_delay;
@@ -1140,6 +1140,13 @@ int igmp6_event_query(struct sk_buff *sk
/* 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)) {
+ in6_dev_put(idev);
+ return -EINVAL;
+ }
+ mlh2 = (struct mld2_query *) skb->h.raw;
max_delay = (MLDV2_MRC(ntohs(mlh2->mrc))*HZ)/1000;
if (!max_delay)
max_delay = 1;
@@ -1156,7 +1163,15 @@ int igmp6_event_query(struct sk_buff *sk
return 0;
}
/* mark sources to include, if group & source-specific */
- mark = mlh2->nsrcs != 0;
+ if (mlh2->nsrcs != 0) {
+ if (!pskb_may_pull(skb, srcs_offset +
+ mlh2->nsrcs * sizeof(struct in6_addr))) {
+ in6_dev_put(idev);
+ return -EINVAL;
+ }
+ mlh2 = (struct mld2_query *) skb->h.raw;
+ mark = 1;
+ }
} else {
in6_dev_put(idev);
return -EINVAL;

2005-10-31 12:15:17

by YOSHIFUJI Hideaki

Subject: Re: [PATCH][MCAST]IPv6: Check packet size when process Multicast Address and Source Specific Query

In article <[email protected]> (at Mon, 31 Oct 2005 20:09:45 +0800), Yan Zheng <[email protected]> says:

> I hope the new one is correct.

looks ok.
Signed-off-by: YOSHIFUJI Hideaki <[email protected]>

--yoshfuji

2005-10-31 19:42:19

by David Stevens

Subject: Re: [PATCH][MCAST]IPv6: Check packet size when process Multicast Address and Source Specific Query

I think this should be modelled after the equivalent code in IGMPv3.
See igmp_heard_query() in net/ipv4/igmp.c. For ease of maintenance,
the code should be structured exactly the same way, except for
necessary differences, of course.

I haven't seen enough context yet, but I think you need to check
for the query header itself, too (as done in IGMPv3).

I'm reviewing your other patches as well.

+-DLS

2005-11-01 04:20:19

by Yan Zheng

Subject: Re: [PATCH][MCAST]IPv6: Check packet size when process Multicast Address and Source Specific Query

> I think this should be modelled after the equivalent code in IGMPv3.
> See igmp_heard_query() in net/ipv4/igmp.c. For ease of maintenance,
> the code should be structured exactly the same way, except for
> necessary differences, of course.
>
> I haven't seen enough context yet, but I think you need to check
> for the query header itself, too (as done in IGMPv3).
>
> I'm reviewing your other patches as well.
>
> +-DLS

Yes . It's better to drop invalid query earlier.