| Message ID | 1610890456-42846-1-git-send-email-wangyingjie55@126.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] ipv4: add iPv4_is_multicast() check in ip_mc_leave_group(). | expand |
On Sun, 17 Jan 2021 05:34:16 -0800 wangyingjie55@126.com wrote: > From: Yingjie Wang <wangyingjie55@126.com> > > There is no iPv4_is_multicast() check added to ip_mc_leave_group() > to check if imr->imr_multiaddr.s_addr is a multicast address. > If not a multicast address, it may result in an error. Could you please say more? From looking at the code it seems like no address should match if group is non-mcast, and -EADDRNOTAVAIL will be returned. Adding Nik to CC. > In some cases, the callers of ip_mc_leave_group() don't check > whether it is multicast address or not before calling > such as do_ip_setsockopt(). So I suggest adding the ipv4_is_multicast() > check to the ip_mc_leave_group() to prevent this from happening. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Yingjie Wang <wangyingjie55@126.com> > --- > net/ipv4/igmp.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c > index 7b272bbed2b4..1b6f91271cfd 100644 > --- a/net/ipv4/igmp.c > +++ b/net/ipv4/igmp.c > @@ -2248,6 +2248,9 @@ int ip_mc_leave_group(struct sock *sk, struct ip_mreqn *imr) > u32 ifindex; > int ret = -EADDRNOTAVAIL; > > + if (!ipv4_is_multicast(group)) > + return -EINVAL; > + > ASSERT_RTNL(); > > in_dev = ip_mc_find_dev(net, imr);
On 19/01/2021 06:39, Jakub Kicinski wrote: > On Sun, 17 Jan 2021 05:34:16 -0800 wangyingjie55@126.com wrote: >> From: Yingjie Wang <wangyingjie55@126.com> >> >> There is no iPv4_is_multicast() check added to ip_mc_leave_group() >> to check if imr->imr_multiaddr.s_addr is a multicast address. >> If not a multicast address, it may result in an error. > > Could you please say more? From looking at the code it seems like > no address should match if group is non-mcast, and -EADDRNOTAVAIL > will be returned. > > Adding Nik to CC. > Thanks, and absolutely right. I don't see any point in changing the code, also you are definitely not fixing any bug. >> In some cases, the callers of ip_mc_leave_group() don't check >> whether it is multicast address or not before calling >> such as do_ip_setsockopt(). So I suggest adding the ipv4_is_multicast() >> check to the ip_mc_leave_group() to prevent this from happening. >> >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") >> Signed-off-by: Yingjie Wang <wangyingjie55@126.com> >> --- >> net/ipv4/igmp.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c >> index 7b272bbed2b4..1b6f91271cfd 100644 >> --- a/net/ipv4/igmp.c >> +++ b/net/ipv4/igmp.c >> @@ -2248,6 +2248,9 @@ int ip_mc_leave_group(struct sock *sk, struct ip_mreqn *imr) >> u32 ifindex; >> int ret = -EADDRNOTAVAIL; >> >> + if (!ipv4_is_multicast(group)) >> + return -EINVAL; >> + >> ASSERT_RTNL(); >> >> in_dev = ip_mc_find_dev(net, imr); >
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c index 7b272bbed2b4..1b6f91271cfd 100644 --- a/net/ipv4/igmp.c +++ b/net/ipv4/igmp.c @@ -2248,6 +2248,9 @@ int ip_mc_leave_group(struct sock *sk, struct ip_mreqn *imr) u32 ifindex; int ret = -EADDRNOTAVAIL; + if (!ipv4_is_multicast(group)) + return -EINVAL; + ASSERT_RTNL(); in_dev = ip_mc_find_dev(net, imr);