| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5db21508-711e-4534-ac71-f6f6c8d12d56@redhat.com>
Date: Tue, 17 Dec 2024 11:24:53 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Kuniyuki Iwashima <kuniyu@...zon.com>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Simon Horman <horms@...nel.org>
Cc: Kuniyuki Iwashima <kuni1840@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH v3 net-next 04/15] socket: Pass hold_net to struct
net_proto_family.create().
On 12/13/24 10:21, Kuniyuki Iwashima wrote:
> We will introduce a new API to create a kernel socket with netns refcnt
> held. Then, sk_alloc() needs the hold_net flag passed to __sock_create().
>
> Let's pass it down to net_proto_family.create() and functions that call
> sk_alloc().
>
> While at it, we convert the kern flag to boolean.
>
> Note that we still need to pass hold_net to struct pppox_proto.create()
> and struct nfc_protocol.create() before passing hold_net to sk_alloc().
>
> Also, we use !kern as hold_net in the accept() paths. We will add the
> hold_net flag to struct proto_accept_arg later.
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com>
> ---
> crypto/af_alg.c | 2 +-
> drivers/isdn/mISDN/socket.c | 13 ++++++++-----
> drivers/net/ppp/pppox.c | 2 +-
> include/linux/net.h | 2 +-
> include/net/bluetooth/bluetooth.h | 3 ++-
> include/net/llc_conn.h | 2 +-
> net/appletalk/ddp.c | 2 +-
> net/atm/common.c | 3 ++-
> net/atm/common.h | 3 ++-
> net/atm/pvc.c | 4 ++--
> net/atm/svc.c | 8 ++++----
> net/ax25/af_ax25.c | 2 +-
> net/bluetooth/af_bluetooth.c | 7 ++++---
> net/bluetooth/bnep/sock.c | 5 +++--
> net/bluetooth/cmtp/sock.c | 2 +-
> net/bluetooth/hci_sock.c | 4 ++--
> net/bluetooth/hidp/sock.c | 5 +++--
> net/bluetooth/iso.c | 11 ++++++-----
> net/bluetooth/l2cap_sock.c | 14 ++++++++------
> net/bluetooth/rfcomm/sock.c | 12 +++++++-----
> net/bluetooth/sco.c | 11 ++++++-----
> net/caif/caif_socket.c | 2 +-
> net/can/af_can.c | 2 +-
> net/ieee802154/socket.c | 2 +-
> net/ipv4/af_inet.c | 2 +-
> net/ipv6/af_inet6.c | 2 +-
> net/iucv/af_iucv.c | 11 ++++++-----
> net/kcm/kcmsock.c | 2 +-
> net/key/af_key.c | 2 +-
> net/llc/af_llc.c | 6 ++++--
> net/llc/llc_conn.c | 9 ++++++---
> net/mctp/af_mctp.c | 2 +-
> net/netlink/af_netlink.c | 8 ++++----
> net/netrom/af_netrom.c | 2 +-
> net/nfc/af_nfc.c | 2 +-
> net/packet/af_packet.c | 2 +-
> net/phonet/af_phonet.c | 2 +-
> net/qrtr/af_qrtr.c | 2 +-
> net/rds/af_rds.c | 2 +-
> net/rose/af_rose.c | 2 +-
> net/rxrpc/af_rxrpc.c | 2 +-
> net/smc/af_smc.c | 15 ++++++++-------
> net/socket.c | 2 +-
> net/tipc/socket.c | 6 ++++--
> net/unix/af_unix.c | 9 +++++----
> net/vmw_vsock/af_vsock.c | 8 ++++----
> net/x25/af_x25.c | 13 ++++++++-----
> net/xdp/xsk.c | 2 +-
> 48 files changed, 133 insertions(+), 105 deletions(-)
The diffstat here and in patch 8/15 is IMHO scareful.
I'm wondering if we could make this more palatable? Can we let
_sock_create() directly acquire the netns reference for kern socket -
when asked? something alike:
---
diff --git a/net/socket.c b/net/socket.c
index 16402b8be5a7..23092f7576cf 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1577,6 +1577,13 @@ int __sock_create(struct net *net, int family,
int type, int protocol,
goto out_module_put;
}
+ DEBUG_NET_WARN_ON_ONCE(!kern && !hold_net);
+ if (hold_net && kern) {
+ sk->sk_net_refcnt = true;
+ get_net_track(net, &sf->sk->ns_tracker, GFP_KERNEL);
+ sock_inuse_add(net, 1);
+ }
+
/*
* Now to bump the refcnt of the [loadable] module that owns this
* socket at sock_release time we decrement its refcnt.
---
(completely untested, just to explain my thoughts). The goal would be to
drop patch 4 & 8 entirely.
Thanks,
Paolo
Powered by blists - more mailing lists