All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH can-next v4 0/3] Introduce diag support for CAN
@ 2026-11-13 11:14 Filippo Storniolo
  2026-11-13 11:14 ` [PATCH can-next v4 1/3] can: ensure sk_protocol is always set on socket creation Filippo Storniolo
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Filippo Storniolo @ 2026-11-13 11:14 UTC (permalink / raw)
  To: Oliver Hartkopp, Marc Kleine-Budde, Robin van der Gracht,
	Oleksij Rempel, kernel, Urs Thuermann, Paolo Abeni, Simon Horman
  Cc: linux-can, Davide Caratti, Filippo Storniolo

In the current state of the art, it is not possible for userspace to use
tool like ss(8) to query open AF_CAN sockets.

This patch series adds the netlink can_diag interface for AF_CAN. Userspace
is  now able to send a netlink request to request information about open
AF_CAN sockets that are retrieved by the can-diag.ko module.

Patch 1 is a fix that is mandatory to the correct behaviour of the diag
module: the field sk_protocol is not assigned during `can_create()` instead
of being assigned only in case the CAN socket belong to the j1939 transport
protocol. This is needed to the correct filtering of the can diag module.

Patch 2 introduces a list per network namespace containing open CAN sockets.
The list is protected by a mutex.
CAN sockets are added in `can_create()` to the list and removed from it
in the `release()` calls defined for each transport protocol.

Patch 3 is the core patch that introduces the userspace structures to query
and receive netlink messages for CAN sockets and the actual CAN diag module.
The latter will use the userspace structures defined in <linux/can/diag.h>
to filter and retrieve open CAN sockets.

In the current implementation, every CAN sockets in the list is
returned to the userspace, however a filtering could be implemented:
Userspace could possibly request only bound CAN sockets or CAN sockets
belonging to a specific transport protocol (raw, iso-tp, j1939, bcm)
and so on.

An example of the output given by ss -cQpe:

Netid       Local Address:Port  Peer Address:Port  Process                                                                 
CAN_ISOTP           vcan1:1110         vcan1:291   uid:1001 ino:50324 sk:0 users:(("can2",pid=9210,fd=4))                 
CAN_ISOTP           vcan1:291          vcan1:1110  uid:1001 ino:50323 sk:0 users:(("can2",pid=9210,fd=3))

---
Changes in v4:
- Rebased to latest net-next
- Implemented context-analysis and tested using clang-23
  (Marc Kleine-Budde)
- Commit messages now always target "can:"
  (Marc Kleine-Budde)
- Removed "depends on CAN" in Kconfig (Marc Kleine-Budde)
- CAN diag now uses `name_t` and `pgn_t` instead of
  `__u64` and `__u32` (Marc Kleine-Budde)
- Added my Signed-off-by on the 1st commit
  (Marc Kleine-Budde)
- Link to v3: https://lore.kernel.org/r/20260904-feat-can-diag-v3-0-bdcebf07b373@redhat.com

Changes in v3:
- Rebased to latest net-next
- Changed no-op functions from #define to static inline
  (Marc Kleine-Budde)
- In struct can_diag_req, sdiag_protocol changed from
  __u8 to __u16. Padding changed accordingly
- Added	CDIAG_SHOW_UID filter in UAPI header and implemented
  filter as netlink attribute in diag module.
- Split sockaddr_can into individual netlink attributes
  for isotp and j1939 respectively, instead of binary-encoding
  it into a single attribute (Sashiko-bot / Marc Kleine-Budde)
- Fixed indentation in /netns/can.h (Marc Kleine-Budde)
- Link to v2: https://lore.kernel.org/r/20260806-feat-can-diag-v2-0-832597ee4cb9@redhat.com

Changes in v2:
- Rebased to latest net-next
- Added a patch, suggested by sashiko-bot, that fixes an attempt
   to lock an uninitialized mutex
- Link to v1: https://lore.kernel.org/r/20260610-feat-can-diag-v1-0-021e3f1631a0@redhat.com

Change w.r.t. RFC:
- header relocation, from linux/can_diag.h to linux/can/diag.h
  (Oliver Hartkopp)
- file renaming, from net/can/can-diag.c to net/can/diag.c
  (Oliver Hartkopp)
- guard renaming, from _UAPI__CAN_DIAG_H__ to _UAPI_CAN_DIAG_H_
  (Oliver Hartkopp)
- handled J1939 addressing in sockaddr_can
  (Oliver Hartkopp)
- Link to RFC: https://lore.kernel.org/linux-can/20260402-feat-can-diag-v1-0-245b56434c1b@redhat.com/

---
Davide Caratti (1):
      can: ensure sk_protocol is always set on socket creation

Filippo Storniolo (2):
      can: store socket pointers in struct netns_can
      can: add can diag interface

 MAINTAINERS                   |   1 +
 include/linux/can/core.h      |  11 +++
 include/net/netns/can.h       |   6 ++
 include/uapi/linux/can/diag.h |  50 +++++++++++
 net/can/Kconfig               |   9 ++
 net/can/Makefile              |   5 ++
 net/can/af_can.c              |  54 +++++++++++
 net/can/bcm.c                 |   2 +
 net/can/diag.c                | 204 ++++++++++++++++++++++++++++++++++++++++++
 net/can/isotp.c               |   2 +
 net/can/j1939/socket.c        |   3 +-
 net/can/raw.c                 |   2 +
 12 files changed, 348 insertions(+), 1 deletion(-)
---
base-commit: 573af801552999954caa8347e4490b04cf4763f0
change-id: 20260529-feat-can-diag-d7acbf5e22c2

Best regards,
-- 
Filippo Storniolo <fstornio@redhat.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH can-next v4 1/3] can: ensure sk_protocol is always set on socket creation
  2026-11-13 11:14 [PATCH can-next v4 0/3] Introduce diag support for CAN Filippo Storniolo
@ 2026-11-13 11:14 ` Filippo Storniolo
  2026-11-13 11:14 ` [PATCH can-next v4 2/3] can: store socket pointers in struct netns_can Filippo Storniolo
  2026-11-13 11:14 ` [PATCH can-next v4 3/3] can: add can diag interface Filippo Storniolo
  2 siblings, 0 replies; 6+ messages in thread
From: Filippo Storniolo @ 2026-11-13 11:14 UTC (permalink / raw)
  To: Oliver Hartkopp, Marc Kleine-Budde, Robin van der Gracht,
	Oleksij Rempel, kernel, Urs Thuermann, Paolo Abeni, Simon Horman
  Cc: linux-can, Davide Caratti, Filippo Storniolo

From: Davide Caratti <dcaratti@redhat.com>

Currently, only j1939 assigns a value to 'sk_protocol'. As a prerequisite
for the implementation of CAN sockets diagnostics, AF_CAN sockets need an
easy way to determine which protocol is on top of an existing socket.

POC test using can-tests:

 | # perf probe -m can_j1939 --add "j1939_sk_recvmsg sock->sk->sk_protocol"
 | # perf record -e probe:j1939_sk_recvmsg -aR -- ./j1939/tst-j1939-ac
 | [...]
 | # perf script
 | tst-j1939-ac  5807 [002] 322767.312599: probe:j1939_sk_recvmsg: (ffffffffc0b29a14) sk_protocol=0x7
 |  ^^^ 0x7, that's CAN_J1939

 | # perf probe -m can_raw --add "raw_recvmsg sock->sk->sk_protocol"
 | # perf record -e probe:raw_recvmsg -aR ./netlayer/tst-rcv-own-msgs vcan0
 | # perf script
 | [...]
 | tst-rcv-own-msg  5816 [001] 323173.651122: probe:raw_recvmsg: (ffffffffc0b20154) sk_protocol=0x1
 |  ^^^ 0x1, that's CAN_RAW. It was 0x0 on unpatched kernel

Storing the "protocol" value in can_create() fixes the problem. This also
fixes AF_CAN support for the following system call:

 | getsockopt(..., SOL_SOCKET, SO_PROTOCOL, &proto, sizeof(proto))

that was assigning 0 to 'proto' since the earliest kernel versions.

Fixes: 0d66548a10cb ("[CAN]: Add PF_CAN core module")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Signed-off-by: Filippo Storniolo <fstornio@redhat.com>
---
 net/can/af_can.c       | 1 +
 net/can/j1939/socket.c | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/can/af_can.c b/net/can/af_can.c
index 0f3051a61aab..b4255fc570aa 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -162,6 +162,7 @@ static int can_create(struct net *net, struct socket *sock, int protocol,
 	}
 
 	sock_init_data(sock, sk);
+	sk->sk_protocol = protocol;
 	sk->sk_destruct = can_sock_destruct;
 
 	if (sk->sk_prot->init)
diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
index 50a598ef5fd4..ccd43ff5519c 100644
--- a/net/can/j1939/socket.c
+++ b/net/can/j1939/socket.c
@@ -420,7 +420,6 @@ static int j1939_sk_init(struct sock *sk)
 	/* j1939_sk_sock_destruct() depends on SOCK_RCU_FREE flag */
 	sock_set_flag(sk, SOCK_RCU_FREE);
 	sk->sk_destruct = j1939_sk_sock_destruct;
-	sk->sk_protocol = CAN_J1939;
 
 	return 0;
 }

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH can-next v4 2/3] can: store socket pointers in struct netns_can
  2026-11-13 11:14 [PATCH can-next v4 0/3] Introduce diag support for CAN Filippo Storniolo
  2026-11-13 11:14 ` [PATCH can-next v4 1/3] can: ensure sk_protocol is always set on socket creation Filippo Storniolo
@ 2026-11-13 11:14 ` Filippo Storniolo
  2026-11-13 11:14 ` [PATCH can-next v4 3/3] can: add can diag interface Filippo Storniolo
  2 siblings, 0 replies; 6+ messages in thread
From: Filippo Storniolo @ 2026-11-13 11:14 UTC (permalink / raw)
  To: Oliver Hartkopp, Marc Kleine-Budde, Robin van der Gracht,
	Oleksij Rempel, kernel, Urs Thuermann, Paolo Abeni, Simon Horman
  Cc: linux-can, Filippo Storniolo

AF_CAN sockets need to be stored in the netns_can structure
in order to be retrieved by the CAN diagnostic module when
a netlink request message is issued by the userspace.

On socket creation (`can_create()`), add the pointer to the
new socket to `netns_can::sk_list`. During socket release
(`isotp_release()`, `raw_release()`, `j1939_release()`,
`bcm_release()`), remove the corresponding pointer from
this list.

Since this is a prerequisite of the CAN diagnostic module,
deletes and insert operations are conditioned by
IS_ENABLED(CONFIG_CAN_DIAG).

Signed-off-by: Filippo Storniolo <fstornio@redhat.com>
---
 include/linux/can/core.h | 11 ++++++++++
 include/net/netns/can.h  |  6 ++++++
 net/can/Makefile         |  2 ++
 net/can/af_can.c         | 53 ++++++++++++++++++++++++++++++++++++++++++++++++
 net/can/bcm.c            |  2 ++
 net/can/isotp.c          |  2 ++
 net/can/j1939/socket.c   |  2 ++
 net/can/raw.c            |  2 ++
 8 files changed, 80 insertions(+)

diff --git a/include/linux/can/core.h b/include/linux/can/core.h
index effb5c31ef40..ae3cfa537ea8 100644
--- a/include/linux/can/core.h
+++ b/include/linux/can/core.h
@@ -62,4 +62,15 @@ extern int can_send(struct sk_buff *skb, int loop);
 void can_set_skb_uid(struct can_skb_ext *csx);
 void can_sock_destruct(struct sock *sk);
 
+/* function prototypes for the CAN diag module */
+#if IS_ENABLED(CONFIG_CAN_DIAG)
+void lock_can_diag_mutex(struct net *net) __acquires(&net->can.sklist_lock);
+void unlock_can_diag_mutex(struct net *net) __releases(&net->can.sklist_lock);
+void can_add_sock_sklist(struct sock *sk);
+void can_remove_sock_sklist(struct sock *sk);
+#else
+static inline void can_add_sock_sklist(struct sock *sk) {};
+static inline void can_remove_sock_sklist(struct sock *sk) {};
+#endif
+
 #endif /* !_CAN_CORE_H */
diff --git a/include/net/netns/can.h b/include/net/netns/can.h
index 54f23681e0d4..691dcb808b3a 100644
--- a/include/net/netns/can.h
+++ b/include/net/netns/can.h
@@ -36,6 +36,12 @@ struct netns_can {
 
 	/* CAN GW per-net gateway jobs */
 	struct hlist_head cgw_list;
+
+#if IS_ENABLED(CONFIG_CAN_DIAG)
+	/* CAN diag support */
+	struct mutex sklist_lock;
+	struct hlist_head sklist __guarded_by(&sklist_lock);
+#endif
 };
 
 #endif /* __NETNS_CAN_H__ */
diff --git a/net/can/Makefile b/net/can/Makefile
index 58f2c31c1ef3..7116c8c7f331 100644
--- a/net/can/Makefile
+++ b/net/can/Makefile
@@ -3,6 +3,8 @@
 #  Makefile for the Linux Controller Area Network core.
 #
 
+CONTEXT_ANALYSIS := y
+
 obj-$(CONFIG_CAN)	+= can.o
 can-y			:= af_can.o
 can-$(CONFIG_PROC_FS)	+= proc.o
diff --git a/net/can/af_can.c b/net/can/af_can.c
index b4255fc570aa..6ee035fc14af 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -111,6 +111,46 @@ static inline void can_put_proto(const struct can_proto *cp)
 	module_put(cp->prot->owner);
 }
 
+#if IS_ENABLED(CONFIG_CAN_DIAG)
+void lock_can_diag_mutex(struct net *net)
+	__acquires(&net->can.sklist_lock)
+{
+	mutex_lock(&net->can.sklist_lock);
+}
+EXPORT_SYMBOL(lock_can_diag_mutex);
+
+void unlock_can_diag_mutex(struct net *net)
+	__releases(&net->can.sklist_lock)
+{
+	mutex_unlock(&net->can.sklist_lock);
+}
+EXPORT_SYMBOL(unlock_can_diag_mutex);
+
+void can_add_sock_sklist(struct sock *sk)
+{
+	struct net *net;
+
+	net = sock_net(sk);
+
+	lock_can_diag_mutex(net);
+	sk_add_node(sk, &net->can.sklist);
+	unlock_can_diag_mutex(net);
+}
+EXPORT_SYMBOL(can_add_sock_sklist);
+
+void can_remove_sock_sklist(struct sock *sk)
+{
+	struct net *net;
+
+	net = sock_net(sk);
+
+	lock_can_diag_mutex(net);
+	sk_del_node_init(sk);
+	unlock_can_diag_mutex(net);
+}
+EXPORT_SYMBOL(can_remove_sock_sklist);
+#endif
+
 static int can_create(struct net *net, struct socket *sock, int protocol,
 		      int kern)
 {
@@ -174,6 +214,8 @@ static int can_create(struct net *net, struct socket *sock, int protocol,
 		sock_put(sk);
 		sock->sk = NULL;
 	} else {
+		can_add_sock_sklist(sk);
+
 		sock_prot_inuse_add(net, sk->sk_prot, 1);
 	}
 
@@ -831,6 +873,12 @@ EXPORT_SYMBOL(can_proto_unregister);
 static int can_pernet_init(struct net *net)
 {
 	spin_lock_init(&net->can.rcvlists_lock);
+
+#if IS_ENABLED(CONFIG_CAN_DIAG)
+	scoped_guard(mutex_init, &net->can.sklist_lock)
+		INIT_HLIST_HEAD(&net->can.sklist);
+#endif
+
 	net->can.rx_alldev_list = kzalloc_obj(*net->can.rx_alldev_list);
 	if (!net->can.rx_alldev_list)
 		goto out;
@@ -875,6 +923,11 @@ static void can_pernet_exit(struct net *net)
 	kfree(net->can.rx_alldev_list);
 	kfree(net->can.pkg_stats);
 	kfree(net->can.rcv_lists_stats);
+
+#if IS_ENABLED(CONFIG_CAN_DIAG)
+	scoped_guard(mutex, &net->can.sklist_lock)
+		WARN_ON_ONCE(!hlist_empty(&net->can.sklist));
+#endif
 }
 
 /* af_can module init/exit functions */
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 7bd2aa39777a..512faa7a62b6 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1910,6 +1910,8 @@ static int bcm_release(struct socket *sock)
 	if (!sk)
 		return 0;
 
+	can_remove_sock_sklist(sk);
+
 	net = sock_net(sk);
 	bo = bcm_sk(sk);
 
diff --git a/net/can/isotp.c b/net/can/isotp.c
index 7a90ba3f1cf1..1141c4757082 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -1468,6 +1468,8 @@ static int isotp_release(struct socket *sock)
 	so = isotp_sk(sk);
 	net = sock_net(sk);
 
+	can_remove_sock_sklist(sk);
+
 	/* best-effort: wait for a running pdu to finish, but don't block on
 	 * it forever - give up after the first signal
 	 */
diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
index ccd43ff5519c..cbfd0b888768 100644
--- a/net/can/j1939/socket.c
+++ b/net/can/j1939/socket.c
@@ -641,6 +641,8 @@ static int j1939_sk_release(struct socket *sock)
 	if (!sk)
 		return 0;
 
+	can_remove_sock_sklist(sk);
+
 	lock_sock(sk);
 	jsk = j1939_sk(sk);
 
diff --git a/net/can/raw.c b/net/can/raw.c
index f14270b56f52..7f99af2548b6 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -442,6 +442,8 @@ static int raw_release(struct socket *sock)
 	ro = raw_sk(sk);
 	net = sock_net(sk);
 
+	can_remove_sock_sklist(sk);
+
 	spin_lock(&raw_notifier_lock);
 	while (raw_busy_notifier == ro) {
 		spin_unlock(&raw_notifier_lock);

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH can-next v4 3/3] can: add can diag interface
  2026-11-13 11:14 [PATCH can-next v4 0/3] Introduce diag support for CAN Filippo Storniolo
  2026-11-13 11:14 ` [PATCH can-next v4 1/3] can: ensure sk_protocol is always set on socket creation Filippo Storniolo
  2026-11-13 11:14 ` [PATCH can-next v4 2/3] can: store socket pointers in struct netns_can Filippo Storniolo
@ 2026-11-13 11:14 ` Filippo Storniolo
  2026-11-13 11:41   ` sashiko-bot
  2026-11-13 12:02   ` Marc Kleine-Budde
  2 siblings, 2 replies; 6+ messages in thread
From: Filippo Storniolo @ 2026-11-13 11:14 UTC (permalink / raw)
  To: Oliver Hartkopp, Marc Kleine-Budde, Robin van der Gracht,
	Oleksij Rempel, kernel, Urs Thuermann, Paolo Abeni, Simon Horman
  Cc: linux-can, Davide Caratti, Filippo Storniolo

Add the can_diag interface for querying sockets from userspace.
ss(8) tool can use this interface to list open sockets.

The userspace ABI is defined in <linux/can/diag.h> and includes
netlink request and response structs.  The request queries open
can sockets and the response contains socket information fields
including the interface index for bound sockets, inode number,
transport protocol etc.

Support can be added later by extending can_diag_dump().

Suggested-by: Davide Caratti <dcaratti@redhat.com>
Signed-off-by: Filippo Storniolo <fstornio@redhat.com>
---
 MAINTAINERS                   |   1 +
 include/uapi/linux/can/diag.h |  50 +++++++++++
 net/can/Kconfig               |   9 ++
 net/can/Makefile              |   3 +
 net/can/diag.c                | 204 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 267 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index c583367c88bc..71e98ca362ad 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6029,6 +6029,7 @@ F:	include/net/can.h
 F:	include/net/netns/can.h
 F:	include/uapi/linux/can.h
 F:	include/uapi/linux/can/bcm.h
+F:	include/uapi/linux/can/diag.h
 F:	include/uapi/linux/can/gw.h
 F:	include/uapi/linux/can/isotp.h
 F:	include/uapi/linux/can/raw.h
diff --git a/include/uapi/linux/can/diag.h b/include/uapi/linux/can/diag.h
new file mode 100644
index 000000000000..f22103e7bf90
--- /dev/null
+++ b/include/uapi/linux/can/diag.h
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+
+#ifndef _UAPI_CAN_DIAG_H_
+#define _UAPI_CAN_DIAG_H_
+
+#include <linux/types.h>
+#include <linux/can.h>
+#include <linux/const.h>
+
+/* Request */
+struct can_diag_req {
+	__u8	sdiag_family;	/* must be AF_CAN */
+	__u8	pad;
+	__u16	sdiag_protocol; /* for future filtering of transport protocols */
+	__u32	cdiag_states;
+	__u32	cdiag_ino;
+	__u32	cdiag_show;
+	__u32	cdiag_cookie[2];
+};
+
+#define CDIAG_SHOW_UID		_BITUL(0) /* show socket's UID */
+
+enum {
+	CAN_DIAG_UNSPEC,
+	CAN_DIAG_UID,
+	CAN_DIAG_ISOTP_TX_ID,
+	CAN_DIAG_ISOTP_RX_ID,
+	CAN_DIAG_J1939_NAME,
+	CAN_DIAG_J1939_PGN,
+	CAN_DIAG_J1939_ADDR,
+
+	__CAN_DIAG_MAX,
+};
+
+#define CAN_DIAG_MAX (__CAN_DIAG_MAX - 1)
+
+/* Response */
+struct can_diag_msg {
+	__u8	cdiag_family;	/* AF_CAN */
+	__u8	cdiag_state;
+	__u16	cdiag_protocol;
+	__u16	cdiag_type;
+	__u16	pad16;
+	__u32	cdiag_cookie[2];
+	__s32	cdiag_ifindex;
+	__u32	pad32;
+	__u64	cdiag_ino;
+};
+
+#endif /* _UAPI_CAN_DIAG_H_ */
diff --git a/net/can/Kconfig b/net/can/Kconfig
index abbb4be7ad21..6766efe83630 100644
--- a/net/can/Kconfig
+++ b/net/can/Kconfig
@@ -70,4 +70,13 @@ config CAN_ISOTP
 	  as needed e.g. for vehicle diagnosis (UDS, ISO 14229) or IP-over-CAN
 	  traffic.
 
+config CAN_DIAG
+	tristate "CAN socket monitoring interface"
+	default m
+	help
+	  Support for CAN socket monitoring interface used by the ss tool.
+	  If unsure, say M.
+
+	  Enable this module so userspace applications can query open sockets.
+
 endif
diff --git a/net/can/Makefile b/net/can/Makefile
index 7116c8c7f331..7c375220c57f 100644
--- a/net/can/Makefile
+++ b/net/can/Makefile
@@ -22,3 +22,6 @@ obj-$(CONFIG_CAN_J1939)	+= j1939/
 
 obj-$(CONFIG_CAN_ISOTP)	+= can-isotp.o
 can-isotp-y		:= isotp.o
+
+obj-$(CONFIG_CAN_DIAG) += can-diag.o
+can-diag-y		:= diag.o
diff --git a/net/can/diag.c b/net/can/diag.c
new file mode 100644
index 000000000000..11281f75ecd5
--- /dev/null
+++ b/net/can/diag.c
@@ -0,0 +1,204 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *
+ * Copyright (C) 2026 Red Hat
+ * Author: Filippo Storniolo <fstornio@redhat.com>
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/net.h>
+#include <net/netlink.h>
+#include <linux/sock_diag.h>
+#include <linux/can.h>
+#include <linux/can/j1939.h>
+#include <linux/can/diag.h>
+#include <net/net_namespace.h>
+#include <net/sock.h>
+#include <linux/netdevice.h>
+#include <linux/user_namespace.h>
+#include <linux/can/core.h>
+
+static int sk_diag_dump_uid(struct sock *sk, struct sk_buff *skb,
+			    struct user_namespace *user_ns)
+{
+	uid_t uid = from_kuid_munged(user_ns, sk_uid(sk));
+
+	return nla_put(skb, CAN_DIAG_UID, sizeof(uid_t), &uid);
+}
+
+static int sk_diag_fill(struct sock *sk, struct sk_buff *skb,
+			struct can_diag_req *req, /* will be used for filtering */
+			struct user_namespace *user_ns,
+			u32 portid, u32 seq, u32 flags, u64 sk_ino)
+{
+	struct sockaddr_can can_addr;
+	struct can_diag_msg *rep;
+	struct nlmsghdr *nlh;
+	int err;
+
+	nlh = nlmsg_put(skb, portid, seq, SOCK_DIAG_BY_FAMILY, sizeof(*rep), flags);
+	if (!nlh)
+		return -EMSGSIZE;
+
+	rep = nlmsg_data(nlh);
+	memset(rep, 0, sizeof(struct can_diag_msg));
+
+	rep->cdiag_family = AF_CAN;
+	rep->cdiag_type = sk->sk_type;
+	rep->cdiag_ino = sk_ino;
+	rep->cdiag_protocol = sk->sk_protocol;
+	rep->cdiag_state = READ_ONCE(sk->sk_state);
+	sock_diag_save_cookie(sk, rep->cdiag_cookie);
+
+	if ((req->cdiag_show & CDIAG_SHOW_UID)) {
+		err = sk_diag_dump_uid(sk, skb, user_ns);
+		if (err < 0)
+			goto cancel_nlmsg_err;
+	}
+
+	memset(&can_addr, 0, sizeof(can_addr));
+
+	err = kernel_getsockname(sk->sk_socket, (struct sockaddr *)&can_addr);
+	if (err < 0) {
+		/* Some protocols (e.g. CAN_BCM) do not implement kernel_getsockname().
+		 * No error returned because the netlink message is still valid.
+		 */
+		if (err == -EOPNOTSUPP)
+			goto exit_no_err;
+
+		goto cancel_nlmsg_err;
+	}
+
+	rep->cdiag_ifindex = can_addr.can_ifindex;
+
+	switch (sk->sk_protocol) {
+	case CAN_ISOTP:
+	{
+		canid_t tx_id;
+		canid_t rx_id;
+
+		tx_id = can_addr.can_addr.tp.tx_id;
+		rx_id = can_addr.can_addr.tp.rx_id;
+
+		err = nla_put(skb, CAN_DIAG_ISOTP_TX_ID, sizeof(canid_t), &tx_id);
+		if (err < 0)
+			goto cancel_nlmsg_err;
+
+		err = nla_put(skb, CAN_DIAG_ISOTP_RX_ID, sizeof(canid_t), &rx_id);
+		if (err < 0)
+			goto cancel_nlmsg_err;
+	}
+		break;
+	case CAN_J1939:
+	{
+		name_t	j1939_name;
+		pgn_t	j1939_pgn;
+		__u8	j1939_addr;
+
+		j1939_name = can_addr.can_addr.j1939.name;
+		j1939_pgn = can_addr.can_addr.j1939.pgn;
+		j1939_addr = can_addr.can_addr.j1939.addr;
+
+		err = nla_put(skb, CAN_DIAG_J1939_NAME, sizeof(j1939_name), &j1939_name);
+		if (err < 0)
+			goto cancel_nlmsg_err;
+
+		err = nla_put(skb, CAN_DIAG_J1939_PGN, sizeof(j1939_pgn), &j1939_pgn);
+		if (err < 0)
+			goto cancel_nlmsg_err;
+
+		err = nla_put(skb, CAN_DIAG_J1939_ADDR, sizeof(j1939_addr), &j1939_addr);
+		if (err < 0)
+			goto cancel_nlmsg_err;
+	}
+		break;
+	default:
+		break;
+	}
+
+exit_no_err:
+	nlmsg_end(skb, nlh);
+	return 0;
+
+cancel_nlmsg_err:
+	nlmsg_cancel(skb, nlh);
+	return err;
+}
+
+static int can_diag_dump(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	int num = 0, s_num = cb->args[0];
+	struct can_diag_req *req;
+	struct net *net;
+	struct sock *sk;
+
+	net = sock_net(skb->sk);
+	req = nlmsg_data(cb->nlh);
+
+	lock_can_diag_mutex(net);
+	sk_for_each(sk, &net->can.sklist) {
+		if (num < s_num)
+			goto next;
+
+		if (sk_diag_fill(sk, skb, req,
+				 sk_user_ns(NETLINK_CB(cb->skb).sk),
+				 NETLINK_CB(cb->skb).portid,
+				 cb->nlh->nlmsg_seq, NLM_F_MULTI,
+				 sock_i_ino(sk)) < 0)
+			goto done;
+next:
+		num++;
+	}
+done:
+	unlock_can_diag_mutex(net);
+	cb->args[0] = num;
+
+	return skb->len;
+}
+
+static int can_diag_handler_dump(struct sk_buff *skb, struct nlmsghdr *h)
+{
+	int hdrlen = sizeof(struct can_diag_req);
+	struct net *net = sock_net(skb->sk);
+	struct can_diag_req *req;
+
+	if (nlmsg_len(h) < hdrlen)
+		return -EINVAL;
+
+	req = nlmsg_data(h);
+	if (req->sdiag_protocol)
+		return -EINVAL;
+
+	if (h->nlmsg_flags & NLM_F_DUMP) {
+		struct netlink_dump_control c = {
+			.dump = can_diag_dump
+		};
+		return netlink_dump_start(net->diag_nlsk, skb, h, &c);
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static const struct sock_diag_handler can_diag_handler = {
+	.owner = THIS_MODULE,
+	.family = AF_CAN,
+	.dump = can_diag_handler_dump,
+};
+
+static int __init can_diag_init(void)
+{
+	pr_info("can: diagnostic module\n");
+	return sock_diag_register(&can_diag_handler);
+}
+
+static void __exit can_diag_exit(void)
+{
+	sock_diag_unregister(&can_diag_handler);
+}
+
+module_init(can_diag_init);
+module_exit(can_diag_exit);
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("CAN socket monitoring via SOCK_DIAG");
+MODULE_ALIAS_NET_PF_PROTO_TYPE(PF_NETLINK, NETLINK_SOCK_DIAG, AF_CAN);

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH can-next v4 3/3] can: add can diag interface
  2026-11-13 11:14 ` [PATCH can-next v4 3/3] can: add can diag interface Filippo Storniolo
@ 2026-11-13 11:41   ` sashiko-bot
  2026-11-13 12:02   ` Marc Kleine-Budde
  1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-11-13 11:41 UTC (permalink / raw)
  To: Filippo Storniolo
  Cc: linux-can, Marc Kleine-Budde, Oliver Hartkopp, Vincent Mailhol,
	Oleksij Rempel

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] can: diag: unaligned 64-bit J1939_NAME and cdiag_ino in
  sk_diag_fill()
--

commit 6641e4d72c858dd57e3276451bf5e3299a63d8d2
Author: Filippo Storniolo <fstornio@redhat.com>

can: add can diag interface

This commit adds a SOCK_DIAG netlink interface and UAPI definitions for
monitoring open AF_CAN sockets from userspace.

> diff --git a/net/can/diag.c b/net/can/diag.c
> new file mode 100644
> index 0000000000000..11281f75ecd52
> --- /dev/null
> +++ b/net/can/diag.c
[ ... ]
> +static int sk_diag_fill(struct sock *sk, struct sk_buff *skb,
> +			struct can_diag_req *req, /* will be used for filtering */
> +			struct user_namespace *user_ns,
> +			u32 portid, u32 seq, u32 flags, u64 sk_ino)
> +{
> +	struct sockaddr_can can_addr;
> +	struct can_diag_msg *rep;
> +	struct nlmsghdr *nlh;
> +	int err;
> +
> +	nlh = nlmsg_put(skb, portid, seq, SOCK_DIAG_BY_FAMILY, sizeof(*rep), flags);
> +	if (!nlh)
> +		return -EMSGSIZE;
> +
> +	rep = nlmsg_data(nlh);
> +	memset(rep, 0, sizeof(struct can_diag_msg));
> +
> +	rep->cdiag_family = AF_CAN;
> +	rep->cdiag_type = sk->sk_type;
> +	rep->cdiag_ino = sk_ino;
[ ... ]
> +	case CAN_J1939:
> +	{
> +		name_t	j1939_name;
> +		pgn_t	j1939_pgn;
> +		__u8	j1939_addr;
> +
> +		j1939_name = can_addr.can_addr.j1939.name;
> +		j1939_pgn = can_addr.can_addr.j1939.pgn;
> +		j1939_addr = can_addr.can_addr.j1939.addr;
> +
> +		err = nla_put(skb, CAN_DIAG_J1939_NAME, sizeof(j1939_name), &j1939_name);

[Severity: Medium]
Should CAN_DIAG_J1939_NAME be emitted with nla_put_u64_64bit() instead of
nla_put()?

struct nlmsghdr (16 bytes), struct can_diag_msg (32 bytes), and the optional
CAN_DIAG_UID attribute (8 bytes) leave the attribute stream 8-byte aligned.
Because struct nlattr is 4 bytes, nla_put() places the 8-byte j1939_name
payload at a 4-byte-aligned offset and leaves the finished netlink message
length at a multiple of 4 rather than 8 (after the 8-byte CAN_DIAG_J1939_PGN
and padded 8-byte CAN_DIAG_J1939_ADDR attributes).

When can_diag_dump() packs another socket message into the same skb after a
CAN_J1939 socket, nlmsg_put() aligns only to NLMSG_ALIGNTO (4 bytes), so the
next struct can_diag_msg starts at an unaligned offset and rep->cdiag_ino =
sk_ino performs an unaligned 64-bit write in the kernel and unaligned reads
in userspace.  Using nla_put_u64_64bit() with a pad attribute and marking
cdiag_ino as __aligned_u64 in struct can_diag_msg would keep both the
attribute and subsequent messages aligned before the UAPI is frozen.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-feat-can-diag-v4-0-c28842c74c95@redhat.com?part=3

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH can-next v4 3/3] can: add can diag interface
  2026-11-13 11:14 ` [PATCH can-next v4 3/3] can: add can diag interface Filippo Storniolo
  2026-11-13 11:41   ` sashiko-bot
@ 2026-11-13 12:02   ` Marc Kleine-Budde
  1 sibling, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2026-11-13 12:02 UTC (permalink / raw)
  To: Filippo Storniolo
  Cc: Oliver Hartkopp, Robin van der Gracht, Oleksij Rempel, kernel,
	Urs Thuermann, Paolo Abeni, Simon Horman, Davide Caratti,
	linux-can

[-- Attachment #1: Type: text/plain, Size: 3355 bytes --]

On 13.11.2026 12:14:35, Filippo Storniolo wrote:
> +	memset(&can_addr, 0, sizeof(can_addr));
> +
> +	err = kernel_getsockname(sk->sk_socket, (struct sockaddr *)&can_addr);
> +	if (err < 0) {
> +		/* Some protocols (e.g. CAN_BCM) do not implement kernel_getsockname().
> +		 * No error returned because the netlink message is still valid.
> +		 */
> +		if (err == -EOPNOTSUPP)
> +			goto exit_no_err;
> +
> +		goto cancel_nlmsg_err;
> +	}
> +
> +	rep->cdiag_ifindex = can_addr.can_ifindex;
> +
> +	switch (sk->sk_protocol) {
> +	case CAN_ISOTP:
> +	{
> +		canid_t tx_id;
> +		canid_t rx_id;
> +
> +		tx_id = can_addr.can_addr.tp.tx_id;
> +		rx_id = can_addr.can_addr.tp.rx_id;
> +
> +		err = nla_put(skb, CAN_DIAG_ISOTP_TX_ID, sizeof(canid_t), &tx_id);
> +		if (err < 0)
> +			goto cancel_nlmsg_err;
> +
> +		err = nla_put(skb, CAN_DIAG_ISOTP_RX_ID, sizeof(canid_t), &rx_id);
> +		if (err < 0)
> +			goto cancel_nlmsg_err;
> +	}
> +		break;
> +	case CAN_J1939:
> +	{
> +		name_t	j1939_name;
> +		pgn_t	j1939_pgn;
> +		__u8	j1939_addr;
> +

Since you re-spin: Nitpick: Don't align with spaces.

> +		j1939_name = can_addr.can_addr.j1939.name;
> +		j1939_pgn = can_addr.can_addr.j1939.pgn;
> +		j1939_addr = can_addr.can_addr.j1939.addr;
> +
> +		err = nla_put(skb, CAN_DIAG_J1939_NAME, sizeof(j1939_name), &j1939_name);
> +		if (err < 0)
> +			goto cancel_nlmsg_err;
> +
> +		err = nla_put(skb, CAN_DIAG_J1939_PGN, sizeof(j1939_pgn), &j1939_pgn);
> +		if (err < 0)
> +			goto cancel_nlmsg_err;
> +
> +		err = nla_put(skb, CAN_DIAG_J1939_ADDR, sizeof(j1939_addr), &j1939_addr);
> +		if (err < 0)
> +			goto cancel_nlmsg_err;
> +	}
> +		break;
> +	default:
> +		break;
> +	}
> +
> +exit_no_err:
> +	nlmsg_end(skb, nlh);
> +	return 0;
> +
> +cancel_nlmsg_err:
> +	nlmsg_cancel(skb, nlh);
> +	return err;
> +}
> +
> +static int can_diag_dump(struct sk_buff *skb, struct netlink_callback *cb)
> +{
> +	int num = 0, s_num = cb->args[0];
> +	struct can_diag_req *req;
> +	struct net *net;
> +	struct sock *sk;
> +
> +	net = sock_net(skb->sk);
> +	req = nlmsg_data(cb->nlh);
> +
> +	lock_can_diag_mutex(net);
> +	sk_for_each(sk, &net->can.sklist) {
> +		if (num < s_num)
> +			goto next;
> +
> +		if (sk_diag_fill(sk, skb, req,
> +				 sk_user_ns(NETLINK_CB(cb->skb).sk),
> +				 NETLINK_CB(cb->skb).portid,
> +				 cb->nlh->nlmsg_seq, NLM_F_MULTI,
> +				 sock_i_ino(sk)) < 0)
> +			goto done;
> +next:
> +		num++;
> +	}
> +done:
> +	unlock_can_diag_mutex(net);
> +	cb->args[0] = num;
> +
> +	return skb->len;
> +}
> +
> +static int can_diag_handler_dump(struct sk_buff *skb, struct nlmsghdr *h)
> +{
> +	int hdrlen = sizeof(struct can_diag_req);
> +	struct net *net = sock_net(skb->sk);
> +	struct can_diag_req *req;
> +
> +	if (nlmsg_len(h) < hdrlen)
> +		return -EINVAL;
> +
> +	req = nlmsg_data(h);
> +	if (req->sdiag_protocol)
> +		return -EINVAL;
> +
> +	if (h->nlmsg_flags & NLM_F_DUMP) {

Nitpick:

invert the condition and exit early with -EOPNOTSUPP here.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-11-13 12:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-13 11:14 [PATCH can-next v4 0/3] Introduce diag support for CAN Filippo Storniolo
2026-11-13 11:14 ` [PATCH can-next v4 1/3] can: ensure sk_protocol is always set on socket creation Filippo Storniolo
2026-11-13 11:14 ` [PATCH can-next v4 2/3] can: store socket pointers in struct netns_can Filippo Storniolo
2026-11-13 11:14 ` [PATCH can-next v4 3/3] can: add can diag interface Filippo Storniolo
2026-11-13 11:41   ` sashiko-bot
2026-11-13 12:02   ` Marc Kleine-Budde

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.