Page MenuHomeFreeBSD

netinet6: Fix two issues with passing v4-mapped groups to IPv6 sockets.
ClosedPublic

Authored by bms on Sun, Aug 2, 1:14 PM.

Details

Summary

EFAULT was happening because inp_join_group() was seeing the user-space thread descriptor in the faked-up sockopt. Prefer nullity to elide to a KVA memcpy(). SMAP would have obscured the panic otherwise expected.

imr_interface was fine, but the unnecessary cast back to its own type in the IA_SIN() macro was obfuscating things. in6_v6_mreq_to_v4() itself was just fine, it was the inputs which were in question.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bms requested review of this revision.Sun, Aug 2, 1:14 PM
sys/netinet6/in6_mcast.c
1927

Spot the obvious typo. Fix incoming. Most likely a squash will be needed.

sys/netinet6/in6_mcast.c
1927

This htonl() may not even be needed at all; if memory serves, the ia_addr member is the *odd man out*.

In actual fact, https://reviews.freebsd.org/D58591 was informative, but the swizzle was happening in the *wrong place*.

It seem it is necessary to byte swap the *group address* before hand-off to the IPv4 sockopt dispatch, which, if memory serves, was not needed in xnu.

I think this is a wrap.

Can you please just squash this revision into D58585? It is a straight one liner bug fix of it.

sys/netinet6/in6_mcast.c
1995

You can just delete this line with a sparse initializer, compiler will nullify.

Can you please just squash this revision into D58585? It is a straight one liner bug fix of it.

Minor garden rake. The commits are now non-consecutive on my pending changes branch, so I'll have to do more rebase busywork to deal with that...

sys/netinet6/in6_mcast.c
1995

Done but not rebased or squashed in just yet.

OK, so the parent revision got pushed by me accidentally already. Squashing at this point is probably out of the question, because rebasing in that way on main would be Very Bad.

My fault, sorry, forgot -i when rebasing onto main for other commits I had pending which wouldn't have required an approval, and I didn't notice that I had pushed the other commits until I saw the "git status" output in my worktree reporting 2 and not 7 ahead of main.

It's up to @glebius and @imp how we deal with this. Having just revisited my Git upskilling notes, git 2.54 introduced "git history", but this may still be too churny to deal with the fallout on main.

On the bright side, everything that did get pushed, was already functionally tested, but on amd64 under BHyve only, although with a "real" NIC (mlx5en(4) SR-IOV VFs) -- with the exception of the copyin issue which is still pending herein, which might panic !amd64, but only if someone expressly triggers this with an IPv4 join on an IPv6 socket.

Taking glebius' requested style changes.

This revision was not accepted when it landed; it landed in state Needs Review.Mon, Aug 3, 3:48 PM
This revision was automatically updated to reflect the committed changes.