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?
> 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) {
Thanks,
Greg
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
