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

Kees Cook posted 7 patches 7 months ago
There is a newer version of this series
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(-)
[PATCH 0/7] net: Convert dev_set_mac_address() to struct sockaddr_storage
Posted by Kees Cook 7 months ago
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(-)

-- 
2.34.1
Re: [PATCH 0/7] net: Convert dev_set_mac_address() to struct sockaddr_storage
Posted by Kuniyuki Iwashima 7 months ago
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.
Re: [PATCH 0/7] net: Convert dev_set_mac_address() to struct sockaddr_storage
Posted by Kees Cook 7 months ago
On Tue, May 20, 2025 at 05:19:20PM -0700, Kuniyuki Iwashima wrote:
> 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.

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 ...
:P

-- 
Kees Cook
Re: [PATCH 0/7] net: Convert dev_set_mac_address() to struct sockaddr_storage
Posted by Jakub Kicinski 7 months ago
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 :(
-- 
pw-bot: cr
Re: [PATCH 0/7] net: Convert dev_set_mac_address() to struct sockaddr_storage
Posted by Kees Cook 7 months ago

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...


-- 
Kees Cook