Re: Patches fixing unaligned access in the networking code

[mlelstv%serpens.de@localhost (Michael van Elst)]

n54%gmx.com@localhost (Kamil Rytarowski) writes:

>Please review:
>http://netbsd.org/~kamil/patch-00115-tcp_input.2.txt
>http://netbsd.org/~kamil/patch-00117-netinet6.txt

>Both patches contain the original messages from kUBSan.
>The first patch is rather obvious, but the 2nd one is unclear for me.

Why would the first be obvious? The TCP header is already aligned
and so is the first option value. I am pretty sure that worse things
will happen if that assumption were broken.

The patch therefore exists to work around a false positive. I wonder
what will be detected when you replace (line 1340):

                optp = ((u_int8_t *)th) + sizeof(struct tcphdr);

with

		optp = (u_int8_t *)(th + 1);

Apparently this is handled better by the sanitizer (lines 4405ff).


>In in6.h, we are following the approach from in.h and we are adding
>__packed to in6_addr struct, reproducing the code for struct in_addr.

This on the other hand is obvious. The structure is already "packed"
(in reality, the C spec would allow something else). Adding __packed
just ensures that no compiler thinks differently and that the sanitizer
can safely assume that it won't.


-- 
-- 
                                Michael van Elst
Internet: mlelstv%serpens.de@localhost
                                "A potential Snark may lurk in every tree."