netdev - Re: [PATCH net-next 16/27] can: raw: instantly reject unsupported CAN frames

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMZ6RqL_nGszwoLPXn1Li8op-ox4k3Hs6p=Hw6+w0W=DTtobPw@mail.gmail.com>
Date: Thu, 27 Nov 2025 20:49:49 +0100
From: Vincent Mailhol <mailhol@...nel.org>
To: Marc Kleine-Budde <mkl@...gutronix.de>, Oliver Hartkopp <socketcan@...tkopp.net>
Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org, 
	linux-can@...r.kernel.org, kernel@...gutronix.de
Subject: Re: [PATCH net-next 16/27] can: raw: instantly reject unsupported CAN frames

On Wed. 26 Nov. 2025 at 13:01, Marc Kleine-Budde <mkl@...gutronix.de> wrote:
> From: Oliver Hartkopp <socketcan@...tkopp.net>
>
> For real CAN interfaces the CAN_CTRLMODE_FD and CAN_CTRLMODE_XL control
> modes indicate whether an interface can handle those CAN FD/XL frames.
>
> In the case a CAN XL interface is configured in CANXL-only mode with
> disabled error-signalling neither CAN CC nor CAN FD frames can be sent.
>
> The checks are performed on CAN_RAW sockets to give an instant feedback
> to the user when writing unsupported CAN frames to the interface.
>
> Signed-off-by: Oliver Hartkopp <socketcan@...tkopp.net>
> Link: https://patch.msgid.link/20251126-canxl-v8-16-e7e3eb74f889@pengutronix.de
> Signed-off-by: Marc Kleine-Budde <mkl@...gutronix.de>
> ---
>  net/can/raw.c | 54 +++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 46 insertions(+), 8 deletions(-)
>
> diff --git a/net/can/raw.c b/net/can/raw.c
> index f36a83d3447c..be1ef7cf4204 100644
> --- a/net/can/raw.c
> +++ b/net/can/raw.c
> @@ -892,20 +892,58 @@ static void raw_put_canxl_vcid(struct raw_sock *ro, struct sk_buff *skb)
>         }
>  }
>
> -static unsigned int raw_check_txframe(struct raw_sock *ro, struct sk_buff *skb, int mtu)
> +static inline bool raw_dev_cc_enabled(struct net_device *dev,
> +                                     struct can_priv *priv)
>  {
> -       /* Classical CAN -> no checks for flags and device capabilities */
> -       if (can_is_can_skb(skb))
> +       /* The CANXL-only mode disables error-signalling on the CAN bus
> +        * which is needed to send CAN CC/FD frames
> +        */
> +       if (priv)
> +               return !can_dev_in_xl_only_mode(priv);
> +
> +       /* virtual CAN interfaces always support CAN CC */
> +       return true;
> +}
> +
> +static inline bool raw_dev_fd_enabled(struct net_device *dev,
> +                                     struct can_priv *priv)
> +{
> +       /* check FD ctrlmode on real CAN interfaces */
> +       if (priv)
> +               return (priv->ctrlmode & CAN_CTRLMODE_FD);
> +
> +       /* check MTU for virtual CAN FD interfaces */
> +       return (READ_ONCE(dev->mtu) >= CANFD_MTU);
> +}
> +
> +static inline bool raw_dev_xl_enabled(struct net_device *dev,
> +                                     struct can_priv *priv)
> +{
> +       /* check XL ctrlmode on real CAN interfaces */
> +       if (priv)
> +               return (priv->ctrlmode & CAN_CTRLMODE_XL);
> +
> +       /* check MTU for virtual CAN XL interfaces */
> +       return can_is_canxl_dev_mtu(READ_ONCE(dev->mtu));
> +}
> +
> +static unsigned int raw_check_txframe(struct raw_sock *ro, struct sk_buff *skb,
> +                                     struct net_device *dev)
> +{
> +       struct can_priv *priv = safe_candev_priv(dev);

Sorry for coming back to you too late after the series is already
merged in net-next.

This dependency on safe_candev_priv() can break the kernel build.
Indeed, when building the kernel with:

  CONFIG_CAN_RAW=y

and with CONFIG_CAN_DEV not set (or built as module) below build error occurs:

  ld: vmlinux.o: in function `raw_sendmsg':
  raw.c:(.text+0x101ac4b): undefined reference to `safe_candev_priv'

This is because, under the current design, the CAN network layer is
not supposed to depend on the CAN devices layer.

I do not have a fix for this, nor do I have time to work on such a
fix. All I can do for the moment is escalate the issue (but it is just
a matter of time before some build bot from linux-next would report
the same issue).

> +       /* Classical CAN */
> +       if (can_is_can_skb(skb) && raw_dev_cc_enabled(dev, priv))
>                 return CAN_MTU;
>
> -       /* CAN FD -> needs to be enabled and a CAN FD or CAN XL device */
> +       /* CAN FD */
>         if (ro->fd_frames && can_is_canfd_skb(skb) &&
> -           (mtu == CANFD_MTU || can_is_canxl_dev_mtu(mtu)))
> +           raw_dev_fd_enabled(dev, priv))
>                 return CANFD_MTU;
>
> -       /* CAN XL -> needs to be enabled and a CAN XL device */
> +       /* CAN XL */
>         if (ro->xl_frames && can_is_canxl_skb(skb) &&
> -           can_is_canxl_dev_mtu(mtu))
> +           raw_dev_xl_enabled(dev, priv))
>                 return CANXL_MTU;
>
>         return 0;
> @@ -961,7 +999,7 @@ static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
>         err = -EINVAL;
>
>         /* check for valid CAN (CC/FD/XL) frame content */
> -       txmtu = raw_check_txframe(ro, skb, READ_ONCE(dev->mtu));
> +       txmtu = raw_check_txframe(ro, skb, dev);
>         if (!txmtu)
>                 goto free_skb;


Yours sincerely,
Vincent Mailhol