On Tue, Dec 11, 2018 at 08:19:55PM +0000, Gregory Smith wrote:
> Hi Ben,
>
> Thanks for taking a look.
>
> Ben Pfaff wrote:
> > This code looks pretty optimistic about things that I'm not sure it
> > should depend on. It uses ALIGNED_CAST in multiple places although I
> > don't see a reason to assume that the data in question is actually
> > aligned.
>
> Right. I should've spent a little more time thinking this through before
> submitting the revision. I've apparently rotted my brain on x86
> development...
>
> As for the fix: are the get_unaligned_xxx() functions the right tool to
> use? I looked through pinctrl.c for other examples of unaligned access
> to mimic, and I was suprised not to find any special handling (e.g., the
> extraction of the 4-byte iaid in pinctrl_handle_put_dhcpv6_opts(), or
> the 2-byte query_type in pinctrl_handle_dns_lookup()).
>
> Out of curiosity, do we have any test coverage for CPU architectures
> that are sensitive to alignment?
We have a few different strategies. get_unaligned_xxx() is one of them.
Another is get_16aligned_be32() and friends, where we can assume that
data structure are at least aligned on a 16-bit boundary (usually the
case). Occasionally, we use "packed" annotations to avoid having to do
anything special. The latter appears to be the case for the dhcpv6 code
you mention.
We have test coverage mostly through Debian builds, which isn't a great
way because Debian lags far behind master, but it's better than nothing.
> > It also appears to read out the DHCP magic cookie before it checks
> > whether the packet is long enough to contain it.
>
> I think this part was actually correct; the patch advances in_dhcp_ptr
> and does a bounds check before derefercing in_dhcp_cookie.
>
> >> + const ovs_be32 *in_dhcp_cookie =
> >> + ALIGNED_CAST(const ovs_be32 *, in_dhcp_ptr);
> >> + in_dhcp_ptr += sizeof *in_dhcp_cookie;
> >> + if (in_dhcp_ptr > end || *in_dhcp_cookie != magic_cookie) {
You're right, I misread the code.
Thanks,
Ben.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
