During the code review for #14210 , the changing of ip_hdr_length_v6() to return at a minimum sizeof (ip6_t) (the common case of a simple IPv6 header) brought up a concern:
ip_hdr_length_nexthdr_v6 (called by ip_hdr_length_v6()) can fail. This code here in ip_hdr_length_v6 just swallows the error for callers. I believe longer term we should change this so it returns a boolean and the callers get the intermediate there (or we just delete it and call the underlying function directly) allowing those callers to all know this can fail.
I'm categorizing this as a feature-request for now, but unpropagated failures could result in a real bug. Some items to consider:
- If ip_hdr_length_v6 or ip_hdr_length_nexthdr_v6 get called multiple times, it's a waste of packet-traversal and function calls. One could consider, at least on inbound, of caching this in ip_recv_attr_t.
- Sometimes failures should default to a small-and-sane value (like 40 bytes for a vanilla IPv6 header), or indicate a failure such that the packet should get dropped or otherwise handled.