[PATCH RFC 0/5] sockaddr usage removal

Kees Cook posted 5 patches 2 weeks, 5 days ago
drivers/infiniband/hw/erdma/erdma_cm.h        |  4 +-
drivers/infiniband/hw/usnic/usnic_transport.c | 16 +++---
drivers/infiniband/sw/siw/siw_cm.h            |  4 +-
drivers/isdn/mISDN/socket.c                   |  2 +-
drivers/net/ppp/pppoe.c                       |  2 +-
drivers/net/ppp/pptp.c                        |  2 +-
drivers/nvme/host/tcp.c                       |  2 +-
drivers/nvme/target/rdma.c                    |  2 +-
drivers/nvme/target/tcp.c                     |  8 ++-
drivers/scsi/iscsi_tcp.c                      | 18 +++----
drivers/soc/qcom/qmi_interface.c              |  2 +-
drivers/target/iscsi/iscsi_target.c           |  2 +-
drivers/target/iscsi/iscsi_target_login.c     | 51 +++++++++----------
fs/dlm/lowcomms.c                             |  2 +-
fs/nfs/nfs4client.c                           |  4 +-
fs/ocfs2/cluster/tcp.c                        | 25 +++++----
fs/smb/server/connection.h                    |  2 +-
fs/smb/server/mgmt/tree_connect.c             |  2 +-
fs/smb/server/transport_ipc.c                 |  4 +-
fs/smb/server/transport_ipc.h                 |  4 +-
fs/smb/server/transport_tcp.c                 |  6 +--
include/linux/inet.h                          |  2 +-
include/linux/net.h                           |  6 +--
include/linux/socket.h                        |  5 +-
include/linux/sunrpc/clnt.h                   |  2 +-
include/net/inet_common.h                     |  2 +-
include/net/ipv6.h                            |  2 +-
include/net/sock.h                            |  2 +-
net/appletalk/ddp.c                           |  2 +-
net/atm/pvc.c                                 |  2 +-
net/atm/svc.c                                 |  2 +-
net/ax25/af_ax25.c                            |  2 +-
net/bluetooth/hci_sock.c                      |  2 +-
net/bluetooth/iso.c                           |  6 +--
net/bluetooth/l2cap_sock.c                    |  6 +--
net/bluetooth/rfcomm/sock.c                   |  3 +-
net/bluetooth/sco.c                           |  6 +--
net/can/isotp.c                               |  3 +-
net/can/j1939/socket.c                        |  2 +-
net/can/raw.c                                 |  2 +-
net/core/dev.c                                |  7 ++-
net/core/dev_ioctl.c                          |  2 +-
net/core/rtnetlink.c                          | 12 ++---
net/core/sock.c                               |  4 +-
net/core/utils.c                              |  8 +--
net/ipv4/af_inet.c                            |  2 +-
net/ipv4/arp.c                                |  2 +-
net/ipv6/af_inet6.c                           |  2 +-
net/iucv/af_iucv.c                            |  6 +--
net/l2tp/l2tp_ip.c                            |  2 +-
net/l2tp/l2tp_ip6.c                           |  2 +-
net/l2tp/l2tp_ppp.c                           |  2 +-
net/llc/af_llc.c                              |  2 +-
net/netlink/af_netlink.c                      |  4 +-
net/netrom/af_netrom.c                        |  2 +-
net/nfc/llcp_sock.c                           |  4 +-
net/packet/af_packet.c                        | 21 ++++----
net/phonet/socket.c                           | 10 ++--
net/qrtr/af_qrtr.c                            |  2 +-
net/qrtr/ns.c                                 |  2 +-
net/rds/af_rds.c                              |  2 +-
net/rose/af_rose.c                            |  2 +-
net/sctp/ipv6.c                               |  2 +-
net/smc/af_smc.c                              |  2 +-
net/smc/smc.h                                 |  2 +-
net/smc/smc_clc.c                             |  2 +-
net/socket.c                                  | 10 ++--
net/sunrpc/clnt.c                             |  9 ++--
net/sunrpc/svcsock.c                          |  8 +--
net/sunrpc/xprtsock.c                         |  4 +-
net/tipc/socket.c                             |  2 +-
net/unix/af_unix.c                            |  9 ++--
net/vmw_vsock/af_vsock.c                      |  2 +-
net/x25/af_x25.c                              |  2 +-
security/tomoyo/network.c                     |  3 +-
75 files changed, 189 insertions(+), 193 deletions(-)
[PATCH RFC 0/5] sockaddr usage removal
Posted by Kees Cook 2 weeks, 5 days ago
(I removed the explicit CC list because it was huge...)

Hi,

This is strictly an RFC -- it's not a complete removal of sockaddr at
all, but it explores what's involved. Some things are easy (e.g. the
first 3 patches), and some is very involved (last patch).

For the most part, the proto_ops::getname() switching from sockaddr
to sockaddr_storage is mostly mechanical (and mostly just removes
sockaddr casts). There are, however, cases where we still end up lying
to the compiler about object sizes (in the case where the backing
object is smaller than sockaddr_storage, but large enough to hold the
protocol-specific address). These remain just as safe as they used to
be. :)

I think for getname() (and similar interfaces) we *do* want to use
sockaddr_storage, but there is kind of an argument to instead use
a struct with a flexible array, e.g.:

struct sockaddr_unspec {
        sa_family_t	sa_family;
	char		sa_data[];
};

If this was done, then all these APIs would switch their casts from
"(struct sockaddr *)" to "(struct sockaddr_unspec *)", even though in
most cases the object is actully a struct sockaddr_storage.

What do folks think?

-Kees


Kees Cook (5):
  Revert "net: dev: Convert sa_data to flexible array in struct
    sockaddr"
  net: core: dev.c confirmed to use classic sockaddr
  rtnetlink: do_setlink: Use sockaddr_storage
  net: core: Convert inet_addr_is_any() to sockaddr_storage
  net: Convert proto_ops::getname to sockaddr_storage

 drivers/infiniband/hw/erdma/erdma_cm.h        |  4 +-
 drivers/infiniband/hw/usnic/usnic_transport.c | 16 +++---
 drivers/infiniband/sw/siw/siw_cm.h            |  4 +-
 drivers/isdn/mISDN/socket.c                   |  2 +-
 drivers/net/ppp/pppoe.c                       |  2 +-
 drivers/net/ppp/pptp.c                        |  2 +-
 drivers/nvme/host/tcp.c                       |  2 +-
 drivers/nvme/target/rdma.c                    |  2 +-
 drivers/nvme/target/tcp.c                     |  8 ++-
 drivers/scsi/iscsi_tcp.c                      | 18 +++----
 drivers/soc/qcom/qmi_interface.c              |  2 +-
 drivers/target/iscsi/iscsi_target.c           |  2 +-
 drivers/target/iscsi/iscsi_target_login.c     | 51 +++++++++----------
 fs/dlm/lowcomms.c                             |  2 +-
 fs/nfs/nfs4client.c                           |  4 +-
 fs/ocfs2/cluster/tcp.c                        | 25 +++++----
 fs/smb/server/connection.h                    |  2 +-
 fs/smb/server/mgmt/tree_connect.c             |  2 +-
 fs/smb/server/transport_ipc.c                 |  4 +-
 fs/smb/server/transport_ipc.h                 |  4 +-
 fs/smb/server/transport_tcp.c                 |  6 +--
 include/linux/inet.h                          |  2 +-
 include/linux/net.h                           |  6 +--
 include/linux/socket.h                        |  5 +-
 include/linux/sunrpc/clnt.h                   |  2 +-
 include/net/inet_common.h                     |  2 +-
 include/net/ipv6.h                            |  2 +-
 include/net/sock.h                            |  2 +-
 net/appletalk/ddp.c                           |  2 +-
 net/atm/pvc.c                                 |  2 +-
 net/atm/svc.c                                 |  2 +-
 net/ax25/af_ax25.c                            |  2 +-
 net/bluetooth/hci_sock.c                      |  2 +-
 net/bluetooth/iso.c                           |  6 +--
 net/bluetooth/l2cap_sock.c                    |  6 +--
 net/bluetooth/rfcomm/sock.c                   |  3 +-
 net/bluetooth/sco.c                           |  6 +--
 net/can/isotp.c                               |  3 +-
 net/can/j1939/socket.c                        |  2 +-
 net/can/raw.c                                 |  2 +-
 net/core/dev.c                                |  7 ++-
 net/core/dev_ioctl.c                          |  2 +-
 net/core/rtnetlink.c                          | 12 ++---
 net/core/sock.c                               |  4 +-
 net/core/utils.c                              |  8 +--
 net/ipv4/af_inet.c                            |  2 +-
 net/ipv4/arp.c                                |  2 +-
 net/ipv6/af_inet6.c                           |  2 +-
 net/iucv/af_iucv.c                            |  6 +--
 net/l2tp/l2tp_ip.c                            |  2 +-
 net/l2tp/l2tp_ip6.c                           |  2 +-
 net/l2tp/l2tp_ppp.c                           |  2 +-
 net/llc/af_llc.c                              |  2 +-
 net/netlink/af_netlink.c                      |  4 +-
 net/netrom/af_netrom.c                        |  2 +-
 net/nfc/llcp_sock.c                           |  4 +-
 net/packet/af_packet.c                        | 21 ++++----
 net/phonet/socket.c                           | 10 ++--
 net/qrtr/af_qrtr.c                            |  2 +-
 net/qrtr/ns.c                                 |  2 +-
 net/rds/af_rds.c                              |  2 +-
 net/rose/af_rose.c                            |  2 +-
 net/sctp/ipv6.c                               |  2 +-
 net/smc/af_smc.c                              |  2 +-
 net/smc/smc.h                                 |  2 +-
 net/smc/smc_clc.c                             |  2 +-
 net/socket.c                                  | 10 ++--
 net/sunrpc/clnt.c                             |  9 ++--
 net/sunrpc/svcsock.c                          |  8 +--
 net/sunrpc/xprtsock.c                         |  4 +-
 net/tipc/socket.c                             |  2 +-
 net/unix/af_unix.c                            |  9 ++--
 net/vmw_vsock/af_vsock.c                      |  2 +-
 net/x25/af_x25.c                              |  2 +-
 security/tomoyo/network.c                     |  3 +-
 75 files changed, 189 insertions(+), 193 deletions(-)

-- 
2.34.1
Re: [PATCH RFC 0/5] sockaddr usage removal
Posted by Jakub Kicinski 2 weeks, 4 days ago
On Mon,  4 Nov 2024 14:25:02 -0800 Kees Cook wrote:
> I think for getname() (and similar interfaces) we *do* want to use
> sockaddr_storage, but there is kind of an argument to instead use
> a struct with a flexible array, e.g.:
> 
> struct sockaddr_unspec {
>         sa_family_t	sa_family;
> 	char		sa_data[];
> };
> 
> If this was done, then all these APIs would switch their casts from
> "(struct sockaddr *)" to "(struct sockaddr_unspec *)", even though in
> most cases the object is actully a struct sockaddr_storage.

struct sockaddr_unspec was my knee-jerk reaction but looking at the code
- indeed passing struct sockaddr_storage seems cleaner.

> What do folks think?

Looks nice, and feels like the right direction :)

FWIW if the conversion work is too tedious I think I can find some
people that could help.