linux-kernel - Re: [PATCH v2 07/20] can: netlink: refactor CAN_CTRLMODE_TDC

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: <20250922-amber-spider-of-control-90be7c-mkl@pengutronix.de>
Date: Mon, 22 Sep 2025 11:43:53 +0200
From: Marc Kleine-Budde <mkl@...gutronix.de>
To: Vincent Mailhol <mailhol@...nel.org>
Cc: Oliver Hartkopp <socketcan@...tkopp.net>, 
	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
Subject: Re: [PATCH v2 07/20] can: netlink: refactor
 CAN_CTRLMODE_TDC_{AUTO,MANUAL} flag reset logic

On 20.09.2025 16:24:42, Vincent Mailhol wrote:
> On 10/09/2025 at 15:03, Vincent Mailhol wrote:
> > CAN_CTRLMODE_TDC_AUTO and CAN_CTRLMODE_TDC_MANUAL are mutually
> > exclusive. This means that whenever the user switches from auto to
> > manual mode (or vice versa), the other flag which was set previously
> > needs to be cleared.
> > 
> > Currently, this is handled with a masking operation. It can be done in
> > a simpler manner by clearing any of the previous TDC flags before
> > copying netlink attributes. The code becomes easier to understand and
> > will make it easier to add the new upcoming CAN XL flags which will
> > have a similar reset logic as the current TDC flags.
> > 
> > Signed-off-by: Vincent Mailhol <mailhol@...nel.org>
> > ---
> >  drivers/net/can/dev/netlink.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
> > index 274eaab10796b601d565c32f6315727a578970bb..72a82d4e9d6494771320ea035ed6f6098c0e8ce6 100644
> > --- a/drivers/net/can/dev/netlink.c
> > +++ b/drivers/net/can/dev/netlink.c
> > @@ -254,6 +254,10 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
> >  		if ((maskedflags & ctrlstatic) != ctrlstatic)
> >  			return -EOPNOTSUPP;
> >  
> > +		/* If a top dependency flag is provided, reset all its dependencies */
> > +		if (cm->mask & CAN_CTRLMODE_FD)
> > +			priv->ctrlmode &= !CAN_CTRLMODE_FD_TDC_MASK;
>                                           ^
> 
> This is a bug. The correct operation to unset the flag is:
> 
> 		priv->ctrlmode &= ~CAN_CTRLMODE_FD_TDC_MASK;
> 
> (replace the ! operator by ~).
> 
> @Marc, do you think you can send your next PR to net soonish?
> 
> I would like to rebase this series and the "rework the CAN MTU logic" series on
> top of the MTU fix:
> 
> https://lore.kernel.org/linux-can/20250918-can-fix-mtu-v1-0-0d1cada9393b@kernel.org/
> 
> But to do so, I first need to wait for the MTU fix to appear on net-next and
> there is not so much time left before the end of the development windows.

This series looks fine to me. After -rc1, please check for any
ndo_change_mtu, because the Nuvoton CAN-FD driver will go mainline, but
not via the net-next tree.

> If the schedule is too short, let me know and I will adjust accordingly by
> dropping whatever patches are in conflict.

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   |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)