Bugzilla – Bug 1568
Deserialized Addresses are implicitly marked as Mac48Address
Last modified: 2013-03-11 18:22:49 EDT
The Address class, once deserialized, can only be converted in a Mac48Address, as any other compatibility check will fail. This problem affects how a shorter (or longer) address is read from an "ARP-like" protocol, as the Address will not have a proper "Type" (Type 0 is Mac48) and the Length will be depending on the padding. the "ARP-like" protocol deserialization doesn't know anything about the padding, so the address length is the maximum address length, instead of the actual address length.
Created attachment 1500 [details] Bug fix (minimal changes option) This change fixes the issue, although it does allow a longer address to be converted in a shorter one, i.e., a Mac48 into a Mac16 (not viceversa). Another option would be to reserve a type for the "unknown" address (the deserialized one), or to add a bool flag with the same purpose. This approach, however, would be way more complex than the previous one and, tbh, I can't figure out when it would be necessary since the Address deserialization is basically handled only by ARP-like caches (ARP and Ndisc), and those are attached to a specific NetDevice, hence restricting the addresses to just one kind. Well, that's not totally true, as 802.15.4 does have TWO addresses, however the cache will work only on one address kind, unless one is crazy enough to use both 64 and 16 bit addresses on the same PAN with IPv6 running on both, which is a crazy idea. Nevertheless, I could come up with a cleaner patch, if necessary, but it would be more complex.
I checked and the "other option" is indeed already in place. Every address has a "Type" (e.g., Mac48Address::GetType()) starting from 1, whereas the type 0 is reserved for unknown addresses. Hence, the proposed solution is indeed the only possible one. Rationale of the proposed change: when an Address is deserialized from a packet it could have a length greater than its size due to the padding. The proposed change allows to convert an unknown address to any address that is shorter, provided that its type is unknown. In other terms: suppose that an address is carried in an Icmpv6OptionLinkLayerAddress, which has a padding of x bits. Due to the particular padding (it's bit padding), a Mac16 and a Mac48 address will *both* be deserialized in an Address which is 6 bytes long. Without the patch the Mac16 can not be converted. A Mac64 will, viceversa, would be converted only in a Mac112 (which doesn't exist). Any more elaborate (and foolproof) solution would imply to assume how the padding is applied, which is not possible as it's protocol-dependent. The drawbacks of the patch are: it is possible to misinterpret a Mac address. On the other hand this shouldn't be possible as a network will usually use only one Mac address kind. The only (to my best knowledge) net that does that is 802.15.4, however IPv6 is used only with one kind of address, not both. Hence, the problem shouldn't hit.
OK, can you also add a comment for future maintainer's sake such as // Mac address type/length detection is discussed in bug 1568
(In reply to comment #3) > OK, can you also add a comment for future maintainer's sake such as > > // Mac address type/length detection is discussed in bug 1568 Of course. If George is ok with the patch I'll push it with the comment in the code. T.
Fixed in changeset 9251:7c3f81b720a7
Actually fixed in changeset 9252:9fe4804d5c25 Plus, the applied change is a bit more stringent on the requirements (only a just deserialized address can be longer). The actual test is: (m_len == len && m_type == type) || (m_len >= len && m_type == 0) T.