| Message ID | 20220510172739.30823-1-tariqt@nvidia.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: ping6: Fix ping -6 with interface name | expand |
On 5/10/22 11:27 AM, Tariq Toukan wrote: > From: Aya Levin <ayal@nvidia.com> > > When passing interface parameter to ping -6: > $ ping -6 ::11:141:84:9 -I eth2 > Results in: > PING ::11:141:84:10(::11:141:84:10) from ::11:141:84:9 eth2: 56 data bytes > ping: sendmsg: Invalid argument > ping: sendmsg: Invalid argument > > Initialize the fl6's outgoing interface (OIF) before triggering > ip6_datagram_send_ctl. > > Fixes: 13651224c00b ("net: ping6: support setting basic SOL_IPV6 options via cmsg") > Signed-off-by: Aya Levin <ayal@nvidia.com> > Reviewed-by: Gal Pressman <gal@nvidia.com> > Reviewed-by: Saeed Mahameed <saeedm@nvidia.com> > Signed-off-by: Tariq Toukan <tariqt@nvidia.com> > --- > net/ipv6/ping.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/ipv6/ping.c b/net/ipv6/ping.c > index ff033d16549e..83f014559c0d 100644 > --- a/net/ipv6/ping.c > +++ b/net/ipv6/ping.c > @@ -106,6 +106,8 @@ static int ping_v6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len) > > opt.tot_len = sizeof(opt); > ipc6.opt = &opt; > + memset(&fl6, 0, sizeof(fl6)); > + fl6.flowi6_oif = oif; > > err = ip6_datagram_send_ctl(sock_net(sk), sk, msg, &fl6, &ipc6); > if (err < 0) I agree that fl6 is used unitialized here, but right after this is: memset(&fl6, 0, sizeof(fl6)); fl6.flowi6_proto = IPPROTO_ICMPV6; fl6.saddr = np->saddr; fl6.daddr = *daddr; fl6.flowi6_oif = oif; so adding a memset before the call to ip6_datagram_send_ctl duplicates the existing one. Best to move the memset before the 'if (msg->msg_controllen) {'
On 5/10/2022 11:27 PM, David Ahern wrote: > On 5/10/22 11:27 AM, Tariq Toukan wrote: >> From: Aya Levin <ayal@nvidia.com> >> >> When passing interface parameter to ping -6: >> $ ping -6 ::11:141:84:9 -I eth2 >> Results in: >> PING ::11:141:84:10(::11:141:84:10) from ::11:141:84:9 eth2: 56 data bytes >> ping: sendmsg: Invalid argument >> ping: sendmsg: Invalid argument >> >> Initialize the fl6's outgoing interface (OIF) before triggering >> ip6_datagram_send_ctl. >> >> Fixes: 13651224c00b ("net: ping6: support setting basic SOL_IPV6 options via cmsg") >> Signed-off-by: Aya Levin <ayal@nvidia.com> >> Reviewed-by: Gal Pressman <gal@nvidia.com> >> Reviewed-by: Saeed Mahameed <saeedm@nvidia.com> >> Signed-off-by: Tariq Toukan <tariqt@nvidia.com> >> --- >> net/ipv6/ping.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/net/ipv6/ping.c b/net/ipv6/ping.c >> index ff033d16549e..83f014559c0d 100644 >> --- a/net/ipv6/ping.c >> +++ b/net/ipv6/ping.c >> @@ -106,6 +106,8 @@ static int ping_v6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len) >> >> opt.tot_len = sizeof(opt); >> ipc6.opt = &opt; >> + memset(&fl6, 0, sizeof(fl6)); >> + fl6.flowi6_oif = oif; >> >> err = ip6_datagram_send_ctl(sock_net(sk), sk, msg, &fl6, &ipc6); >> if (err < 0) > > I agree that fl6 is used unitialized here, but right after this is: > > memset(&fl6, 0, sizeof(fl6)); > > fl6.flowi6_proto = IPPROTO_ICMPV6; > fl6.saddr = np->saddr; > fl6.daddr = *daddr; > fl6.flowi6_oif = oif; > > so adding a memset before the call to ip6_datagram_send_ctl duplicates > the existing one. Best to move the memset before the 'if > (msg->msg_controllen) {' Hi, Thanks for your comment. As far as I understand the flow, any changes done to fl6 inside ip6_datagram_send_ctl is to be disregarded. That's how I understand the comment: /* Changes to txoptions and flow info are not implemented, yet. * Drop the options, fl6 is wiped below. */ Thanks, Aya
diff --git a/net/ipv6/ping.c b/net/ipv6/ping.c index ff033d16549e..83f014559c0d 100644 --- a/net/ipv6/ping.c +++ b/net/ipv6/ping.c @@ -106,6 +106,8 @@ static int ping_v6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len) opt.tot_len = sizeof(opt); ipc6.opt = &opt; + memset(&fl6, 0, sizeof(fl6)); + fl6.flowi6_oif = oif; err = ip6_datagram_send_ctl(sock_net(sk), sk, msg, &fl6, &ipc6); if (err < 0)