mbox series

[0/7] net: Convert dev_set_mac_address() to struct sockaddr_storage

Message ID 20250520222452.work.063-kees@kernel.org
Headers show
Series net: Convert dev_set_mac_address() to struct sockaddr_storage | expand

Message

Kees Cook May 20, 2025, 10:30 p.m. UTC 
Hi,

As part of the effort to allow the compiler to reason about object sizes,
we need to deal with the problematic variably sized struct sockaddr,
which has no internal runtime size tracking. In much of the network
stack the use of struct sockaddr_storage has been adopted. Continue the
transition toward this for more of the internal APIs. Specifically:

- inet_addr_is_any()
- netif_set_mac_address()
- dev_set_mac_address()

Only 3 callers of dev_set_mac_address() needed adjustment; all others
were already using struct sockaddr_storage internally.

-Kees

Kees Cook (7):
  net: core: Convert inet_addr_is_any() to sockaddr_storage
  net: core: Switch netif_set_mac_address() to struct sockaddr_storage
  net/ncsi: Use struct sockaddr_storage for pending_mac
  ieee802154: Use struct sockaddr_storage with dev_set_mac_address()
  net: usb: r8152: Convert to use struct sockaddr_storage internally
  net: core: Convert dev_set_mac_address() to struct sockaddr_storage
  rtnetlink: do_setlink: Use struct sockaddr_storage

 include/linux/inet.h                |  2 +-
 include/linux/netdevice.h           |  4 +--
 net/ncsi/internal.h                 |  2 +-
 drivers/net/bonding/bond_alb.c      |  8 ++---
 drivers/net/bonding/bond_main.c     | 10 +++---
 drivers/net/hyperv/netvsc_drv.c     |  6 ++--
 drivers/net/macvlan.c               | 10 +++---
 drivers/net/team/team_core.c        |  2 +-
 drivers/net/usb/r8152.c             | 52 +++++++++++++++--------------
 drivers/nvme/target/rdma.c          |  2 +-
 drivers/nvme/target/tcp.c           |  2 +-
 drivers/target/iscsi/iscsi_target.c |  2 +-
 net/core/dev.c                      | 11 +++---
 net/core/dev_api.c                  |  6 ++--
 net/core/rtnetlink.c                | 19 +++--------
 net/core/utils.c                    |  8 ++---
 net/ieee802154/nl-phy.c             |  6 ++--
 net/ncsi/ncsi-rsp.c                 | 18 +++++-----
 18 files changed, 79 insertions(+), 91 deletions(-)

Comments

Kuniyuki Iwashima May 21, 2025, 12:19 a.m. UTC | #1 
From: Kees Cook <kees@kernel.org>
Date: Tue, 20 May 2025 15:30:59 -0700
> Hi,
> 
> As part of the effort to allow the compiler to reason about object sizes,
> we need to deal with the problematic variably sized struct sockaddr,
> which has no internal runtime size tracking. In much of the network
> stack the use of struct sockaddr_storage has been adopted. Continue the
> transition toward this for more of the internal APIs. Specifically:
> 
> - inet_addr_is_any()
> - netif_set_mac_address()
> - dev_set_mac_address()
> 
> Only 3 callers of dev_set_mac_address() needed adjustment; all others
> were already using struct sockaddr_storage internally.

I guess dev_set_mac_address_user() was missed on the way ?

For example, tap_ioctl() still uses sockaddr and calls
dev_set_mac_address_user(), which cast it to _storage.
Jakub Kicinski May 21, 2025, 3:09 a.m. UTC | #2 
On Tue, 20 May 2025 17:42:32 -0700 Kees Cook wrote:
> Ah yes, I can include that in the next version if you want? I was trying
> to find a stopping point since everything kind of touches everything ...

Looks like the build considers -Wincompatible-pointer-types to always
imply -Werror or some such? We explicitly disable CONFIG_WERROR in our
CI, but we still get:

drivers/net/macvlan.c:1302:34: error: incompatible pointer types passing 'struct sockaddr *' to parameter of type 'struct __kernel_sockaddr_storage *' [-Werror,-Wincompatible-pointer-types]
 1302 |                 dev_set_mac_address(port->dev, &sa, NULL);
      |                                                ^~~

on this series :(
Kees Cook May 21, 2025, 5:18 a.m. UTC | #3 
On May 20, 2025 8:09:29 PM PDT, Jakub Kicinski <kuba@kernel.org> wrote:
>On Tue, 20 May 2025 17:42:32 -0700 Kees Cook wrote:
>> Ah yes, I can include that in the next version if you want? I was trying
>> to find a stopping point since everything kind of touches everything ...
>
>Looks like the build considers -Wincompatible-pointer-types to always
>imply -Werror or some such? We explicitly disable CONFIG_WERROR in our
>CI, but we still get:
>
>drivers/net/macvlan.c:1302:34: error: incompatible pointer types passing 'struct sockaddr *' to parameter of type 'struct __kernel_sockaddr_storage *' [-Werror,-Wincompatible-pointer-types]
> 1302 |                 dev_set_mac_address(port->dev, &sa, NULL);
>      |                                                ^~~
>
>on this series :(

I'll get this fixed and add dev_set_mac_address_user() for v3...