rtsock: Avoid copying uninitialized padding bytes

[markj]

Summary

When copying sockaddrs out to userspace, we pad them to a multiple of
the platform alignment (sizeof(long)). However, some sockaddr sizes,
such as struct sockaddr_dl, are not an integer multiple of the
alignment, so we may end up copying out uninitialized bytes.

Fix this by always bouncing through a pre-zeroed sockaddr_storage.

Test Plan

This was found by KASAN.

[melifaro]

Well, I'd prefer to fix it by changing usable length of struct sockaddr_dl to be on 8-byte boundary, as it's actually dynamic.
Effectively the primary use case for it is storing the interface link-level address in the ifp ifaddrs list, so it can be a simple change in link_init_sdl().
The other part, gateway interfaces, already uses struct sockaddr_dl_short which is 16 bytes IIRC.

However, the change above requires more though/testing, so prob it's still good to have the safeguard described here.

If net/routing tests passes, then I'd say it's good to go.

[markj]

In D27729#619842, @melifaro wrote:

Well, I'd prefer to fix it by changing usable length of struct sockaddr_dl to be on 8-byte boundary, as it's actually dynamic.
Effectively the primary use case for it is storing the interface link-level address in the ifp ifaddrs list, so it can be a simple change in link_init_sdl().
The other part, gateway interfaces, already uses struct sockaddr_dl_short which is 16 bytes IIRC.

Wouldn't it also break the userspace ABI?

[melifaro]

In D27729#619862, @markj wrote:

In D27729#619842, @melifaro wrote:

Well, I'd prefer to fix it by changing usable length of struct sockaddr_dl to be on 8-byte boundary, as it's actually dynamic.
Effectively the primary use case for it is storing the interface link-level address in the ifp ifaddrs list, so it can be a simple change in link_init_sdl().
The other part, gateway interfaces, already uses struct sockaddr_dl_short which is 16 bytes IIRC.

Wouldn't it also break the userspace ABI?

Unlikely. E.g. userspace applications will be able to access all necessary fields, size of the new structure will be less, so if they're copying it to an on-stack buffer, it will also work.
I don't currently see any obvious way of how it will break anything, but It's certainly worth testing some of the existing applications.

[markj]

In D27729#619875, @melifaro wrote:

In D27729#619862, @markj wrote:

In D27729#619842, @melifaro wrote:

Well, I'd prefer to fix it by changing usable length of struct sockaddr_dl to be on 8-byte boundary, as it's actually dynamic.
Effectively the primary use case for it is storing the interface link-level address in the ifp ifaddrs list, so it can be a simple change in link_init_sdl().
The other part, gateway interfaces, already uses struct sockaddr_dl_short which is 16 bytes IIRC.

Wouldn't it also break the userspace ABI?

Unlikely. E.g. userspace applications will be able to access all necessary fields, size of the new structure will be less, so if they're copying it to an on-stack buffer, it will also work.

Ah, I thought you were proposing extending it.

I don't currently see any obvious way of how it will break anything, but It's certainly worth testing some of the existing applications.

There are some libc interfaces that use sizeof(struct sockaddr_dl), like getnameinfo(PF_LINK) and link_addr(), but it doesn't look like they'd be broken at least.

[markj]

commit 92be2847e845ba90e4da028cfd7f5a8013919f90