linux-kernel - Re: [PATCH 3/9] can: netlink: add initial CAN XL support

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] [day] [month] [year] [list]

Message-ID: <2145685673.654764160.1763386298170.JavaMail.root@zimbra65-e11.priv.proxad.net>
Date: Mon, 17 Nov 2025 14:31:38 +0100 (CET)
From: Stéphane Grosjean <stephane.grosjean@...e.fr>
To: Vincent Mailhol <mailhol@...nel.org>
Cc: Stéphane Grosjean <stephane.grosjean@...-networks.com>, 
	Robert Nawrath <mbro1689@...il.com>, 
	Minh Le <minh.le.aj@...esas.com>, 
	Duy Nguyen <duy.nguyen.rh@...esas.com>, linux-can@...r.kernel.org, 
	linux-kernel@...r.kernel.org, 
	Oliver Hartkopp <socketcan@...tkopp.net>, 
	Marc Kleine-Budde <mkl@...gutronix.de>
Subject: Re: [PATCH 3/9] can: netlink: add initial CAN XL support

Hi Vincent,

> Then, should we go for this or not? The benefit I see to add this
> check would be
> less effort on the patch reviews for new devices (which I am mostly
> doing
> recently). I can foreseen that people will wrongly use
> CAN_CTRLMODE_LISTENONLY
> instead of CAN_CTRLMODE_RESTRICTED. This piece of code will make sure
> that they
> will implement CAN_CTRLMODE_RESTRICTED. And because this feature is
> mandatory, I
> thought this wasn't a too bad idea. This will also save me the effort
> to ask if
> to people submitting patches if this feature is supported or not.

I understand the benefits for you. But I see the disadvantages for the user. Regardless of whether RESTRICTED mode is mandatory, not requesting this mode should not result in what looks like an error message for the user. As it stands, the fact that it is even a warning is even more confusing, since the interface is configured anyway. And I don't think I'm an average user. Given that the interface will be configured anyway, this warning is more disruptive than useful to me. And that's really a strong opinion.

> If someone comes with a non-compliant device, we can still statically
> set the
> flag. So this isn't even a blocker for non compliant devices.
> 

After reading the very strict interpretation of the ES bit (which I also agree with, despite being the originator of the initial simplification proposal), I admit that I don't understand this:

Speaking of my case: our CANXL core does not currently include RESTRICTED mode (which is considered more of an option to be added in the future). If CANXL grows in popularity, it will be much more due to its finally relevant throughputs and payload length than to this mode, which is close to LISTEN_ONLY in my opinion.

Furthermore, I find it hard to imagine that the RESTRICTED flag would be added statically, whereas in reality, considering it as an option, the driver could add it as a feature finally supported by new firmware versions.

I see it more like CAN_CTRLMODE_CC_LEN8_DLC: if I'm not mistaken, DLC values > 8 have always been allowed by the standard, and yet it was only added in 5.11.

At worst, let's define a flag such as CAN_CTRLMODE_FD_NON_ISO, i.e., CAN_CTRLMODE_XL_NON_RESTRICTED.

> So, at the end, this is mostly a selfish feature to remove some patch
> reviewing
> effort (while still pushing toward ISO compliance, which isn't a bad
> thing either).
> 
> I prefer to have it like this, but this isn't a strong opinion
> either.
> 
> 
> Yours sincerely,
> Vincent Mailhol
> 

Looking forward to hearing from you,

-- Stéphane