* [PATCH can-next v2 0/3] Introduce diag support for CAN
@ 2026-08-06 13:14 Filippo Storniolo
2026-08-06 13:14 ` [PATCH can-next v2 1/3] af_can: ensure sk_protocol is always set on socket creation Filippo Storniolo
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Filippo Storniolo @ 2026-08-06 13: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 -cpe:
Netid Recv-Q Send-Q Local Address:Port Peer Address:Port Process
CAN_ISOTP 0 0 vcan1:1110 vcan1:291 uid:1001 ino:50324 sk:0 users:(("can2",pid=9210,fd=4))
CAN_ISOTP 0 0 vcan1:291 vcan1:1110 uid:1001 ino:50323 sk:0 users:(("can2",pid=9210,fd=3))
---
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):
af_can: ensure sk_protocol is always set on socket creation
Filippo Storniolo (2):
af_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 | 55 +++++++++++++
net/can/Kconfig | 10 +++
net/can/Makefile | 3 +
net/can/af_can.c | 51 ++++++++++++
net/can/bcm.c | 2 +
net/can/diag.c | 185 ++++++++++++++++++++++++++++++++++++++++++
net/can/isotp.c | 2 +
net/can/j1939/socket.c | 3 +-
net/can/raw.c | 2 +
12 files changed, 330 insertions(+), 1 deletion(-)
---
base-commit: 69963a0678a347d57c4ac8b16939dba216eb95ce
change-id: 20260529-feat-can-diag-d7acbf5e22c2
Best regards,
--
Filippo Storniolo <fstornio@redhat.com>
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH can-next v2 1/3] af_can: ensure sk_protocol is always set on socket creation 2026-08-06 13:14 [PATCH can-next v2 0/3] Introduce diag support for CAN Filippo Storniolo @ 2026-08-06 13:14 ` Filippo Storniolo 2026-08-06 13:14 ` [PATCH can-next v2 2/3] af_can: store socket pointers in struct netns_can Filippo Storniolo 2026-08-06 13:14 ` [PATCH can-next v2 3/3] can: add can diag interface Filippo Storniolo 2 siblings, 0 replies; 10+ messages in thread From: Filippo Storniolo @ 2026-08-06 13: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 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> --- 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 7bc86b176b4d..65af25946985 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.55.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH can-next v2 2/3] af_can: store socket pointers in struct netns_can 2026-08-06 13:14 [PATCH can-next v2 0/3] Introduce diag support for CAN Filippo Storniolo 2026-08-06 13:14 ` [PATCH can-next v2 1/3] af_can: ensure sk_protocol is always set on socket creation Filippo Storniolo @ 2026-08-06 13:14 ` Filippo Storniolo 2026-08-06 14:16 ` Marc Kleine-Budde 2026-08-06 13:14 ` [PATCH can-next v2 3/3] can: add can diag interface Filippo Storniolo 2 siblings, 1 reply; 10+ messages in thread From: Filippo Storniolo @ 2026-08-06 13: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/af_can.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ net/can/bcm.c | 2 ++ net/can/isotp.c | 2 ++ net/can/j1939/socket.c | 2 ++ net/can/raw.c | 2 ++ 7 files changed, 75 insertions(+) diff --git a/include/linux/can/core.h b/include/linux/can/core.h index 3287232e3cad..5132f5c89b7c 100644 --- a/include/linux/can/core.h +++ b/include/linux/can/core.h @@ -61,4 +61,15 @@ extern int can_send(struct sk_buff *skb, int loop); void can_set_skb_uid(struct sk_buff *skb); 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); +void unlock_can_diag_mutex(struct net *net); +void can_add_sock_sklist(struct sock *sk); +void can_remove_sock_sklist(struct sock *sk); +#else +#define can_add_sock_sklist(sk) +#define can_remove_sock_sklist(sk) +#endif + #endif /* !_CAN_CORE_H */ diff --git a/include/net/netns/can.h b/include/net/netns/can.h index 48b79f7e6236..bcafff5e6669 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; +#endif }; #endif /* __NETNS_CAN_H__ */ diff --git a/net/can/af_can.c b/net/can/af_can.c index 65af25946985..1ea36411f219 100644 --- a/net/can/af_can.c +++ b/net/can/af_can.c @@ -111,6 +111,44 @@ 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) +{ + mutex_lock(&net->can.sklist_lock); +} +EXPORT_SYMBOL(lock_can_diag_mutex); + +void unlock_can_diag_mutex(struct net *net) +{ + 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 +212,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); } @@ -799,6 +839,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) + 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; @@ -842,6 +888,10 @@ 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) + 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 3d637a1e0ac1..a90b7aea2869 100644 --- a/net/can/bcm.c +++ b/net/can/bcm.c @@ -1921,6 +1921,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 155530aedce2..25098c5546d2 100644 --- a/net/can/isotp.c +++ b/net/can/isotp.c @@ -1455,6 +1455,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 82d9c0499c95..7784b8fd8d19 100644 --- a/net/can/raw.c +++ b/net/can/raw.c @@ -418,6 +418,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.55.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH can-next v2 2/3] af_can: store socket pointers in struct netns_can 2026-08-06 13:14 ` [PATCH can-next v2 2/3] af_can: store socket pointers in struct netns_can Filippo Storniolo @ 2026-08-06 14:16 ` Marc Kleine-Budde 2026-08-06 15:15 ` Oliver Hartkopp 0 siblings, 1 reply; 10+ messages in thread From: Marc Kleine-Budde @ 2026-08-06 14:16 UTC (permalink / raw) To: Filippo Storniolo Cc: Oliver Hartkopp, Robin van der Gracht, Oleksij Rempel, kernel, Urs Thuermann, Paolo Abeni, Simon Horman, linux-can [-- Attachment #1: Type: text/plain, Size: 2664 bytes --] On 06.08.2026 15:14:14, Filippo Storniolo wrote: > 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/af_can.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ > net/can/bcm.c | 2 ++ > net/can/isotp.c | 2 ++ > net/can/j1939/socket.c | 2 ++ > net/can/raw.c | 2 ++ > 7 files changed, 75 insertions(+) > > diff --git a/include/linux/can/core.h b/include/linux/can/core.h > index 3287232e3cad..5132f5c89b7c 100644 > --- a/include/linux/can/core.h > +++ b/include/linux/can/core.h > @@ -61,4 +61,15 @@ extern int can_send(struct sk_buff *skb, int loop); > void can_set_skb_uid(struct sk_buff *skb); > 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); > +void unlock_can_diag_mutex(struct net *net); > +void can_add_sock_sklist(struct sock *sk); > +void can_remove_sock_sklist(struct sock *sk); > +#else > +#define can_add_sock_sklist(sk) > +#define can_remove_sock_sklist(sk) Please add static inline functions for these. > +#endif > + > #endif /* !_CAN_CORE_H */ > diff --git a/include/net/netns/can.h b/include/net/netns/can.h > index 48b79f7e6236..bcafff5e6669 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; nitpick: Please indent with only 1 space as in the rest of this struct. regards, 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] 10+ messages in thread
* Re: [PATCH can-next v2 2/3] af_can: store socket pointers in struct netns_can 2026-08-06 14:16 ` Marc Kleine-Budde @ 2026-08-06 15:15 ` Oliver Hartkopp 2026-08-06 16:41 ` Marc Kleine-Budde 0 siblings, 1 reply; 10+ messages in thread From: Oliver Hartkopp @ 2026-08-06 15:15 UTC (permalink / raw) To: Marc Kleine-Budde, Filippo Storniolo Cc: Robin van der Gracht, Oleksij Rempel, kernel, Urs Thuermann, Paolo Abeni, Simon Horman, linux-can On 06.08.26 16:16, Marc Kleine-Budde wrote: > On 06.08.2026 15:14:14, Filippo Storniolo wrote: >> 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/af_can.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ >> net/can/bcm.c | 2 ++ >> net/can/isotp.c | 2 ++ >> net/can/j1939/socket.c | 2 ++ >> net/can/raw.c | 2 ++ >> 7 files changed, 75 insertions(+) >> >> diff --git a/include/linux/can/core.h b/include/linux/can/core.h >> index 3287232e3cad..5132f5c89b7c 100644 >> --- a/include/linux/can/core.h >> +++ b/include/linux/can/core.h >> @@ -61,4 +61,15 @@ extern int can_send(struct sk_buff *skb, int loop); >> void can_set_skb_uid(struct sk_buff *skb); >> 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); >> +void unlock_can_diag_mutex(struct net *net); >> +void can_add_sock_sklist(struct sock *sk); >> +void can_remove_sock_sklist(struct sock *sk); >> +#else >> +#define can_add_sock_sklist(sk) >> +#define can_remove_sock_sklist(sk) > > Please add static inline functions for these. AFAIK "inline" is not wanted anymore and checkpatch complains about it. Best regards, Oliver > >> +#endif >> + >> #endif /* !_CAN_CORE_H */ >> diff --git a/include/net/netns/can.h b/include/net/netns/can.h >> index 48b79f7e6236..bcafff5e6669 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; > > nitpick: > Please indent with only 1 space as in the rest of this struct. > > regards, > 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 | ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH can-next v2 2/3] af_can: store socket pointers in struct netns_can 2026-08-06 15:15 ` Oliver Hartkopp @ 2026-08-06 16:41 ` Marc Kleine-Budde 0 siblings, 0 replies; 10+ messages in thread From: Marc Kleine-Budde @ 2026-08-06 16:41 UTC (permalink / raw) To: Oliver Hartkopp Cc: Filippo Storniolo, Robin van der Gracht, Oleksij Rempel, kernel, Urs Thuermann, Paolo Abeni, Simon Horman, linux-can [-- Attachment #1: Type: text/plain, Size: 2649 bytes --] On 06.08.2026 17:15:51, Oliver Hartkopp wrote: > > > On 06.08.26 16:16, Marc Kleine-Budde wrote: > > On 06.08.2026 15:14:14, Filippo Storniolo wrote: > > > 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/af_can.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ > > > net/can/bcm.c | 2 ++ > > > net/can/isotp.c | 2 ++ > > > net/can/j1939/socket.c | 2 ++ > > > net/can/raw.c | 2 ++ > > > 7 files changed, 75 insertions(+) > > > > > > diff --git a/include/linux/can/core.h b/include/linux/can/core.h > > > index 3287232e3cad..5132f5c89b7c 100644 > > > --- a/include/linux/can/core.h > > > +++ b/include/linux/can/core.h > > > @@ -61,4 +61,15 @@ extern int can_send(struct sk_buff *skb, int loop); > > > void can_set_skb_uid(struct sk_buff *skb); > > > 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); > > > +void unlock_can_diag_mutex(struct net *net); > > > +void can_add_sock_sklist(struct sock *sk); > > > +void can_remove_sock_sklist(struct sock *sk); > > > +#else > > > +#define can_add_sock_sklist(sk) > > > +#define can_remove_sock_sklist(sk) > > > > Please add static inline functions for these. > > AFAIK "inline" is not wanted anymore and checkpatch complains about it. I'm only talking about the no-op functions. no-op functions must be "static inline". With defines we don't have any type checking, with static inline no-ops, we do. 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] 10+ messages in thread
* [PATCH can-next v2 3/3] can: add can diag interface 2026-08-06 13:14 [PATCH can-next v2 0/3] Introduce diag support for CAN Filippo Storniolo 2026-08-06 13:14 ` [PATCH can-next v2 1/3] af_can: ensure sk_protocol is always set on socket creation Filippo Storniolo 2026-08-06 13:14 ` [PATCH can-next v2 2/3] af_can: store socket pointers in struct netns_can Filippo Storniolo @ 2026-08-06 13:14 ` Filippo Storniolo 2026-08-06 13:51 ` sashiko-bot 2 siblings, 1 reply; 10+ messages in thread From: Filippo Storniolo @ 2026-08-06 13: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 | 55 +++++++++++++ net/can/Kconfig | 10 +++ net/can/Makefile | 3 + net/can/diag.c | 185 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 254 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 932ea1db048e..178caa25a6d2 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5827,6 +5827,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..7f99f85c4242 --- /dev/null +++ b/include/uapi/linux/can/diag.h @@ -0,0 +1,55 @@ +/* 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> + +/* Request */ +struct can_diag_req { + __u8 sdiag_family; /* must be AF_CAN */ + __u8 sdiag_protocol; /* for future filtering of transport protocols */ + __u16 pad; + __u32 cdiag_states; + __u32 cdiag_ino; + __u32 cdiag_show; + __u32 cdiag_cookie[2]; +}; + +enum { + CAN_DIAG_UNSPEC, + CAN_DIAG_UID, + CAN_DIAG_ISOTP_ADDR, + 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; +}; + +struct can_diag_isotp_addr { + canid_t tx_id; + canid_t rx_id; +}; + +struct can_diag_j1939_addr { + __u64 name; + __u32 pgn; + __u8 addr; +}; + +#endif /* _UAPI_CAN_DIAG_H_ */ diff --git a/net/can/Kconfig b/net/can/Kconfig index abbb4be7ad21..ed210d62da7c 100644 --- a/net/can/Kconfig +++ b/net/can/Kconfig @@ -70,4 +70,14 @@ 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" + depends on CAN + default m + help + Support for CAN socket monitoring interface used by the ss tool. + If unsure, say Y. + + Enable this module so userspace applications can query open sockets. + endif diff --git a/net/can/Makefile b/net/can/Makefile index 58f2c31c1ef3..c0ddeb9a012c 100644 --- a/net/can/Makefile +++ b/net/can/Makefile @@ -20,3 +20,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..e9132ba3f8cc --- /dev/null +++ b/net/can/diag.c @@ -0,0 +1,185 @@ +// 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/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_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; + uid_t uid; + 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); + + uid = from_kuid_munged(user_ns, sk_uid(sk)); + err = nla_put(skb, CAN_DIAG_UID, sizeof(uid_t), &uid); + 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: + { + struct can_diag_isotp_addr isotp_addr; + + memset(&isotp_addr, 0, sizeof(isotp_addr)); + + isotp_addr.tx_id = can_addr.can_addr.tp.tx_id; + isotp_addr.rx_id = can_addr.can_addr.tp.rx_id; + + err = nla_put(skb, CAN_DIAG_ISOTP_ADDR, sizeof(struct can_diag_isotp_addr), + &isotp_addr); + } + break; + case CAN_J1939: + { + struct can_diag_j1939_addr j1939_addr; + + memset(&j1939_addr, 0, sizeof(j1939_addr)); + + j1939_addr.name = can_addr.can_addr.j1939.name; + j1939_addr.pgn = can_addr.can_addr.j1939.pgn; + j1939_addr.addr = can_addr.can_addr.j1939.addr; + + err = nla_put(skb, CAN_DIAG_J1939_ADDR, sizeof(struct can_diag_j1939_addr), + &j1939_addr); + } + break; + default: + break; + } + + if (err < 0) + goto cancel_nlmsg_err; + +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.55.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH can-next v2 3/3] can: add can diag interface 2026-08-06 13:14 ` [PATCH can-next v2 3/3] can: add can diag interface Filippo Storniolo @ 2026-08-06 13:51 ` sashiko-bot 2026-08-06 13:54 ` Marc Kleine-Budde 0 siblings, 1 reply; 10+ messages in thread From: sashiko-bot @ 2026-08-06 13:51 UTC (permalink / raw) To: Filippo Storniolo Cc: Oleksij Rempel, Marc Kleine-Budde, Vincent Mailhol, Oliver Hartkopp, linux-can Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Netlink UAPI attributes embed binary structures with implicit compiler padding -- --- Patch [3]: [PATCH can-next v2 3/3] can: add can diag interface --- commit 98771deaa2e2d4a790b170eb8967f6122234dec8 Author: Filippo Storniolo <fstornio@redhat.com> can: add can diag interface This commit adds the can_diag interface for querying CAN sockets from userspace using the ss tool. It defines the userspace ABI including netlink request and response structures to expose socket properties. > diff --git a/include/uapi/linux/can/diag.h b/include/uapi/linux/can/diag.h > new file mode 100644 > index 0000000000000..7f99f85c42425 > --- /dev/null > +++ b/include/uapi/linux/can/diag.h [ ... ] > +struct can_diag_j1939_addr { > + __u64 name; > + __u32 pgn; > + __u8 addr; > +}; [Severity: High] Does this structure introduce implicit compiler padding? With a 64-bit, 32-bit, and 8-bit field, it appears there are 3 bytes of implicit padding at the end. This can lead to architecture-dependent layout mismatches in the UAPI. Additionally, modern Netlink design rules strongly discourage passing binary structures directly inside attributes. Should these structure members be broken out into individual Netlink attributes to ensure proper validation and future extensibility? > diff --git a/net/can/diag.c b/net/can/diag.c > new file mode 100644 > index 0000000000000..e9132ba3f8cc5 > --- /dev/null > +++ b/net/can/diag.c [ ... ] > +static int sk_diag_fill(struct sock *sk, struct sk_buff *skb, [ ... ] > + case CAN_J1939: > + { > + struct can_diag_j1939_addr j1939_addr; > + > + memset(&j1939_addr, 0, sizeof(j1939_addr)); > + > + j1939_addr.name = can_addr.can_addr.j1939.name; > + j1939_addr.pgn = can_addr.can_addr.j1939.pgn; > + j1939_addr.addr = can_addr.can_addr.j1939.addr; > + > + err = nla_put(skb, CAN_DIAG_J1939_ADDR, sizeof(struct can_diag_j1939_addr), > + &j1939_addr); > + } [Severity: High] Does sending this binary structure directly as a raw attribute prevent proper validation and extensibility? Once this ships in a release, the UAPI is frozen and this layout becomes unfixable. Could this be changed to send the individual fields as separate, strongly-typed attributes instead of embedding the C structure directly? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260806-feat-can-diag-v2-0-832597ee4cb9@redhat.com?part=3 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH can-next v2 3/3] can: add can diag interface 2026-08-06 13:51 ` sashiko-bot @ 2026-08-06 13:54 ` Marc Kleine-Budde 2026-08-07 15:40 ` Filippo Storniolo 0 siblings, 1 reply; 10+ messages in thread From: Marc Kleine-Budde @ 2026-08-06 13:54 UTC (permalink / raw) To: sashiko-bot Cc: Filippo Storniolo, Oleksij Rempel, Vincent Mailhol, Oliver Hartkopp, linux-can [-- Attachment #1: Type: text/plain, Size: 2946 bytes --] On 06.08.2026 13:51:22, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [High] Netlink UAPI attributes embed binary structures with implicit compiler padding > -- > > --- Patch [3]: [PATCH can-next v2 3/3] can: add can diag interface --- > commit 98771deaa2e2d4a790b170eb8967f6122234dec8 > Author: Filippo Storniolo <fstornio@redhat.com> > > can: add can diag interface > > This commit adds the can_diag interface for querying CAN sockets from > userspace using the ss tool. It defines the userspace ABI including netlink > request and response structures to expose socket properties. > > > diff --git a/include/uapi/linux/can/diag.h b/include/uapi/linux/can/diag.h > > new file mode 100644 > > index 0000000000000..7f99f85c42425 > > --- /dev/null > > +++ b/include/uapi/linux/can/diag.h > [ ... ] > > +struct can_diag_j1939_addr { > > + __u64 name; > > + __u32 pgn; > > + __u8 addr; > > +}; > > [Severity: High] > Does this structure introduce implicit compiler padding? With a 64-bit, 32-bit, > and 8-bit field, it appears there are 3 bytes of implicit padding at the end. > This can lead to architecture-dependent layout mismatches in the UAPI. > > Additionally, modern Netlink design rules strongly discourage passing binary > structures directly inside attributes. Should these structure members be broken > out into individual Netlink attributes to ensure proper validation and future > extensibility? > > > diff --git a/net/can/diag.c b/net/can/diag.c > > new file mode 100644 > > index 0000000000000..e9132ba3f8cc5 > > --- /dev/null > > +++ b/net/can/diag.c > [ ... ] > > +static int sk_diag_fill(struct sock *sk, struct sk_buff *skb, > [ ... ] > > + case CAN_J1939: > > + { > > + struct can_diag_j1939_addr j1939_addr; > > + > > + memset(&j1939_addr, 0, sizeof(j1939_addr)); > > + > > + j1939_addr.name = can_addr.can_addr.j1939.name; > > + j1939_addr.pgn = can_addr.can_addr.j1939.pgn; > > + j1939_addr.addr = can_addr.can_addr.j1939.addr; > > + > > + err = nla_put(skb, CAN_DIAG_J1939_ADDR, sizeof(struct can_diag_j1939_addr), > > + &j1939_addr); > > + } > > [Severity: High] > Does sending this binary structure directly as a raw attribute prevent proper > validation and extensibility? > > Once this ships in a release, the UAPI is frozen and this layout becomes > unfixable. Could this be changed to send the individual fields as separate, > strongly-typed attributes instead of embedding the C structure directly? Sounds like a valid point. How is this handled with other diag data? regards, 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] 10+ messages in thread
* Re: [PATCH can-next v2 3/3] can: add can diag interface 2026-08-06 13:54 ` Marc Kleine-Budde @ 2026-08-07 15:40 ` Filippo Storniolo 0 siblings, 0 replies; 10+ messages in thread From: Filippo Storniolo @ 2026-08-07 15:40 UTC (permalink / raw) To: Marc Kleine-Budde Cc: sashiko-bot, Oleksij Rempel, Vincent Mailhol, Oliver Hartkopp, linux-can On Thu, Aug 06, 2026 at 03:54:48PM +0200, Marc Kleine-Budde wrote: >On 06.08.2026 13:51:22, sashiko-bot@kernel.org wrote: >> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: >> - [High] Netlink UAPI attributes embed binary structures with implicit compiler padding >> -- >> >> --- Patch [3]: [PATCH can-next v2 3/3] can: add can diag interface --- >> commit 98771deaa2e2d4a790b170eb8967f6122234dec8 >> Author: Filippo Storniolo <fstornio@redhat.com> >> >> can: add can diag interface >> >> This commit adds the can_diag interface for querying CAN sockets from >> userspace using the ss tool. It defines the userspace ABI including netlink >> request and response structures to expose socket properties. >> >> > diff --git a/include/uapi/linux/can/diag.h b/include/uapi/linux/can/diag.h >> > new file mode 100644 >> > index 0000000000000..7f99f85c42425 >> > --- /dev/null >> > +++ b/include/uapi/linux/can/diag.h >> [ ... ] >> > +struct can_diag_j1939_addr { >> > + __u64 name; >> > + __u32 pgn; >> > + __u8 addr; >> > +}; >> >> [Severity: High] >> Does this structure introduce implicit compiler padding? With a 64-bit, 32-bit, >> and 8-bit field, it appears there are 3 bytes of implicit padding at the end. >> This can lead to architecture-dependent layout mismatches in the UAPI. >> >> Additionally, modern Netlink design rules strongly discourage passing binary >> structures directly inside attributes. Should these structure members be broken >> out into individual Netlink attributes to ensure proper validation and future >> extensibility? >> >> > diff --git a/net/can/diag.c b/net/can/diag.c >> > new file mode 100644 >> > index 0000000000000..e9132ba3f8cc5 >> > --- /dev/null >> > +++ b/net/can/diag.c >> [ ... ] >> > +static int sk_diag_fill(struct sock *sk, struct sk_buff *skb, >> [ ... ] >> > + case CAN_J1939: >> > + { >> > + struct can_diag_j1939_addr j1939_addr; >> > + >> > + memset(&j1939_addr, 0, sizeof(j1939_addr)); >> > + >> > + j1939_addr.name = can_addr.can_addr.j1939.name; >> > + j1939_addr.pgn = can_addr.can_addr.j1939.pgn; >> > + j1939_addr.addr = can_addr.can_addr.j1939.addr; >> > + >> > + err = nla_put(skb, CAN_DIAG_J1939_ADDR, sizeof(struct can_diag_j1939_addr), >> > + &j1939_addr); >> > + } >> >> [Severity: High] >> Does sending this binary structure directly as a raw attribute prevent proper >> validation and extensibility? >> >> Once this ships in a release, the UAPI is frozen and this layout becomes >> unfixable. Could this be changed to send the individual fields as separate, >> strongly-typed attributes instead of embedding the C structure directly? > >Sounds like a valid point. How is this handled with other diag data? Hi Marc, thank you for the review. I understand that with the new netlink guidelines, passing binaries structures through the netlink message is discouraged. In the next version I will change that and I will send the single datafields using netlink attributes. kind regards, Filippo ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-08-07 15:41 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-06 13:14 [PATCH can-next v2 0/3] Introduce diag support for CAN Filippo Storniolo 2026-08-06 13:14 ` [PATCH can-next v2 1/3] af_can: ensure sk_protocol is always set on socket creation Filippo Storniolo 2026-08-06 13:14 ` [PATCH can-next v2 2/3] af_can: store socket pointers in struct netns_can Filippo Storniolo 2026-08-06 14:16 ` Marc Kleine-Budde 2026-08-06 15:15 ` Oliver Hartkopp 2026-08-06 16:41 ` Marc Kleine-Budde 2026-08-06 13:14 ` [PATCH can-next v2 3/3] can: add can diag interface Filippo Storniolo 2026-08-06 13:51 ` sashiko-bot 2026-08-06 13:54 ` Marc Kleine-Budde 2026-08-07 15:40 ` Filippo Storniolo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox