UBSAN: object-size-mismatch in wg_xmit

[Jeffrey Walton]

On Mon, Dec 21, 2020 at 6:24 AM Jason A. Donenfeld <Jason at zx2c4.com> wrote:
>
> Hi Dmitry,
>
> On Mon, Dec 21, 2020 at 10:14 AM Dmitry Vyukov <dvyukov at google.com> wrote:
> > Hi Jason,
> >
> > Thanks for looking into this.
> >
> > Reading clang docs for ubsan:
> >
> > https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html
> > -fsanitize=object-size: An attempt to potentially use bytes which the
> > optimizer can determine are not part of the object being accessed.
> > This will also detect some types of undefined behavior that may not
> > directly access memory, but are provably incorrect given the size of
> > the objects involved, such as invalid downcasts and calling methods on
> > invalid pointers. These checks are made in terms of
> > __builtin_object_size, and consequently may be able to detect more
> > problems at higher optimization levels.
> >
> > From skimming though your description this seems to fall into
> > "provably incorrect given the size of the objects involved".
> > I guess it's one of these cases which trigger undefined behavior and
> > compiler can e.g. remove all of this code assuming it will be never
> > called at runtime and any branches leading to it will always branch in
> > other directions, or something.
>
> Right that sort of makes sense, and I can imagine that in more general
> cases the struct casting could lead to UB. But what has me scratching
> my head is that syzbot couldn't reproduce. The cast happens every
> time. What about that one time was special? Did the address happen to
> fall on the border of a mapping? Is UBSAN non-deterministic as an
> optimization? Or is there actually some mysterious UaF happening with
> my usage of skbs that I shouldn't overlook?

The object size checker depends upon compiler analysis. If the
compiler can determine the destination buffer size, then the compiler
can insert a call to a safer function, like a safer memcpy.

If the compiler cannot determine the destination buffer size, then the
compiler will not insert a call to a safer function. (And Wireguard
won't see the crash).

Note... The object size checker and use of safer functions when the
compiler can determine destination buffer sizes is quasi-automatic use
of the safer memory functions from TR 24731-1. They are the ones the
Glibc folks refuse to provide to developers.

Jeff