Hey Klemens, Theo, On Tue, May 26, 2020 at 2:38 PM Klemens Nanni <k...@openbsd.org> wrote: > > On Tue, May 26, 2020 at 02:23:06PM -0600, Jason A. Donenfeld wrote: > > That's good news that it's working for you now, but I didn't change > > anything within the last 24 hours (you mentioned "yesterday") that > > would seem related to this. So perhaps don't get too excited just yet. > I was surprised, too. Perhaps something went wrong applying all those > single patches from the wireguard-openbsd resository, but if so, I'd > doubt that it would build in the first place... > > So far the interface is stable, but I keep testing. When I find the > time, I'll also take a look at the pieces of code which blew up earlier.
My audit is complete, and every single usage of __packed has been removed. Those never should have been there in the first place. WireGuard was specifically designed for kernels, and I distinctly remember making sure things would be high performance on little mips devices, which necessitated natural struct alignment. And indeed, all of the structs that wireguard uses on the wire have fields that are always aligned to each field's size, so no __packed is ever necessary. This all has been fixed up in the git repo now. With regards to your crash, though, that's a bit more puzzling, and I'd be interested to learn more details. Because these structs are already naturally aligned, the __packed attribute, even with the odd nesting Matt had prior, should have produced all entirely aligned accesses. That makes me think your kaboom was coming from someplace else. One possibility is that you were running the git tree on the two days that I was playing with uint128_t, only to find out that some of openbsd's patches to clang miscalculate stack sizes when they're in use, so that work was shelved for another day and the commits removed; perhaps you were just unlucky? Or you hit some other bug that's lurking. Either way, output from ddb's `bt` would at least be useful. Jason
