Commit 2e6259d8 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge tag 'linux-can-fixes-for-6.15-20250506' of...

Merge tag 'linux-can-fixes-for-6.15-20250506' of git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can

Marc Kleine-Budde says:

====================
pull-request: can 2025-05-06

The first patch is by Antonios Salios and adds a missing
spin_lock_init() to the m_can driver.

The next 3 patches are by me and fix the unregistration order in the
mcp251xfd, rockchip_canfd and m_can driver.

The last patch is by Oliver Hartkopp and fixes RCU and BH
locking/handling in the CAN gw protocol.

* tag 'linux-can-fixes-for-6.15-20250506' of git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can:
  can: gw: fix RCU/BH usage in cgw_create_job()
  can: mcan: m_can_class_unregister(): fix order of unregistration calls
  can: rockchip_canfd: rkcanfd_remove(): fix order of unregistration calls
  can: mcp251xfd: mcp251xfd_remove(): fix order of unregistration calls
  can: mcp251xfd: fix TDC setting for low data bit rates
  can: m_can: m_can_class_allocate_dev(): initialize spin lock on device probe
====================

Link: https://patch.msgid.link/20250506135939.652543-1-mkl@pengutronix.de


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 78cd4083 511e64e1
Loading
Loading
Loading
Loading
+2 −1
Original line number Diff line number Diff line
@@ -2379,6 +2379,7 @@ struct m_can_classdev *m_can_class_allocate_dev(struct device *dev,
	SET_NETDEV_DEV(net_dev, dev);

	m_can_of_parse_mram(class_dev, mram_config_vals);
	spin_lock_init(&class_dev->tx_handling_spinlock);
out:
	return class_dev;
}
@@ -2462,9 +2463,9 @@ EXPORT_SYMBOL_GPL(m_can_class_register);

void m_can_class_unregister(struct m_can_classdev *cdev)
{
	unregister_candev(cdev->net);
	if (cdev->is_peripheral)
		can_rx_offload_del(&cdev->offload);
	unregister_candev(cdev->net);
}
EXPORT_SYMBOL_GPL(m_can_class_unregister);

+1 −1
Original line number Diff line number Diff line
@@ -937,8 +937,8 @@ static void rkcanfd_remove(struct platform_device *pdev)
	struct rkcanfd_priv *priv = platform_get_drvdata(pdev);
	struct net_device *ndev = priv->ndev;

	can_rx_offload_del(&priv->offload);
	rkcanfd_unregister(priv);
	can_rx_offload_del(&priv->offload);
	free_candev(ndev);
}

+33 −9
Original line number Diff line number Diff line
@@ -75,6 +75,24 @@ static const struct can_bittiming_const mcp251xfd_data_bittiming_const = {
	.brp_inc = 1,
};

/* The datasheet of the mcp2518fd (DS20006027B) specifies a range of
 * [-64,63] for TDCO, indicating a relative TDCO.
 *
 * Manual tests have shown, that using a relative TDCO configuration
 * results in bus off, while an absolute configuration works.
 *
 * For TDCO use the max value (63) from the data sheet, but 0 as the
 * minimum.
 */
static const struct can_tdc_const mcp251xfd_tdc_const = {
	.tdcv_min = 0,
	.tdcv_max = 63,
	.tdco_min = 0,
	.tdco_max = 63,
	.tdcf_min = 0,
	.tdcf_max = 0,
};

static const char *__mcp251xfd_get_model_str(enum mcp251xfd_model model)
{
	switch (model) {
@@ -510,8 +528,7 @@ static int mcp251xfd_set_bittiming(const struct mcp251xfd_priv *priv)
{
	const struct can_bittiming *bt = &priv->can.bittiming;
	const struct can_bittiming *dbt = &priv->can.data_bittiming;
	u32 val = 0;
	s8 tdco;
	u32 tdcmod, val = 0;
	int err;

	/* CAN Control Register
@@ -575,11 +592,16 @@ static int mcp251xfd_set_bittiming(const struct mcp251xfd_priv *priv)
		return err;

	/* Transmitter Delay Compensation */
	tdco = clamp_t(int, dbt->brp * (dbt->prop_seg + dbt->phase_seg1),
		       -64, 63);
	val = FIELD_PREP(MCP251XFD_REG_TDC_TDCMOD_MASK,
			 MCP251XFD_REG_TDC_TDCMOD_AUTO) |
		FIELD_PREP(MCP251XFD_REG_TDC_TDCO_MASK, tdco);
	if (priv->can.ctrlmode & CAN_CTRLMODE_TDC_AUTO)
		tdcmod = MCP251XFD_REG_TDC_TDCMOD_AUTO;
	else if (priv->can.ctrlmode & CAN_CTRLMODE_TDC_MANUAL)
		tdcmod = MCP251XFD_REG_TDC_TDCMOD_MANUAL;
	else
		tdcmod = MCP251XFD_REG_TDC_TDCMOD_DISABLED;

	val = FIELD_PREP(MCP251XFD_REG_TDC_TDCMOD_MASK, tdcmod) |
		FIELD_PREP(MCP251XFD_REG_TDC_TDCV_MASK, priv->can.tdc.tdcv) |
		FIELD_PREP(MCP251XFD_REG_TDC_TDCO_MASK, priv->can.tdc.tdco);

	return regmap_write(priv->map_reg, MCP251XFD_REG_TDC, val);
}
@@ -2083,10 +2105,12 @@ static int mcp251xfd_probe(struct spi_device *spi)
	priv->can.do_get_berr_counter = mcp251xfd_get_berr_counter;
	priv->can.bittiming_const = &mcp251xfd_bittiming_const;
	priv->can.data_bittiming_const = &mcp251xfd_data_bittiming_const;
	priv->can.tdc_const = &mcp251xfd_tdc_const;
	priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
		CAN_CTRLMODE_LISTENONLY | CAN_CTRLMODE_BERR_REPORTING |
		CAN_CTRLMODE_FD | CAN_CTRLMODE_FD_NON_ISO |
		CAN_CTRLMODE_CC_LEN8_DLC;
		CAN_CTRLMODE_CC_LEN8_DLC | CAN_CTRLMODE_TDC_AUTO |
		CAN_CTRLMODE_TDC_MANUAL;
	set_bit(MCP251XFD_FLAGS_DOWN, priv->flags);
	priv->ndev = ndev;
	priv->spi = spi;
@@ -2174,8 +2198,8 @@ static void mcp251xfd_remove(struct spi_device *spi)
	struct mcp251xfd_priv *priv = spi_get_drvdata(spi);
	struct net_device *ndev = priv->ndev;

	can_rx_offload_del(&priv->offload);
	mcp251xfd_unregister(priv);
	can_rx_offload_del(&priv->offload);
	spi->max_speed_hz = priv->spi_max_speed_hz_orig;
	free_candev(ndev);
}
+90 −59
Original line number Diff line number Diff line
@@ -130,7 +130,7 @@ struct cgw_job {
	u32 handled_frames;
	u32 dropped_frames;
	u32 deleted_frames;
	struct cf_mod mod;
	struct cf_mod __rcu *cf_mod;
	union {
		/* CAN frame data source */
		struct net_device *dev;
@@ -459,6 +459,7 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data)
	struct cgw_job *gwj = (struct cgw_job *)data;
	struct canfd_frame *cf;
	struct sk_buff *nskb;
	struct cf_mod *mod;
	int modidx = 0;

	/* process strictly Classic CAN or CAN FD frames */
@@ -506,7 +507,8 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data)
	 * When there is at least one modification function activated,
	 * we need to copy the skb as we want to modify skb->data.
	 */
	if (gwj->mod.modfunc[0])
	mod = rcu_dereference(gwj->cf_mod);
	if (mod->modfunc[0])
		nskb = skb_copy(skb, GFP_ATOMIC);
	else
		nskb = skb_clone(skb, GFP_ATOMIC);
@@ -529,8 +531,8 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data)
	cf = (struct canfd_frame *)nskb->data;

	/* perform preprocessed modification functions if there are any */
	while (modidx < MAX_MODFUNCTIONS && gwj->mod.modfunc[modidx])
		(*gwj->mod.modfunc[modidx++])(cf, &gwj->mod);
	while (modidx < MAX_MODFUNCTIONS && mod->modfunc[modidx])
		(*mod->modfunc[modidx++])(cf, mod);

	/* Has the CAN frame been modified? */
	if (modidx) {
@@ -546,11 +548,11 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data)
		}

		/* check for checksum updates */
		if (gwj->mod.csumfunc.crc8)
			(*gwj->mod.csumfunc.crc8)(cf, &gwj->mod.csum.crc8);
		if (mod->csumfunc.crc8)
			(*mod->csumfunc.crc8)(cf, &mod->csum.crc8);

		if (gwj->mod.csumfunc.xor)
			(*gwj->mod.csumfunc.xor)(cf, &gwj->mod.csum.xor);
		if (mod->csumfunc.xor)
			(*mod->csumfunc.xor)(cf, &mod->csum.xor);
	}

	/* clear the skb timestamp if not configured the other way */
@@ -581,9 +583,20 @@ static void cgw_job_free_rcu(struct rcu_head *rcu_head)
{
	struct cgw_job *gwj = container_of(rcu_head, struct cgw_job, rcu);

	/* cgw_job::cf_mod is always accessed from the same cgw_job object within
	 * the same RCU read section. Once cgw_job is scheduled for removal,
	 * cf_mod can also be removed without mandating an additional grace period.
	 */
	kfree(rcu_access_pointer(gwj->cf_mod));
	kmem_cache_free(cgw_cache, gwj);
}

/* Return cgw_job::cf_mod with RTNL protected section */
static struct cf_mod *cgw_job_cf_mod(struct cgw_job *gwj)
{
	return rcu_dereference_protected(gwj->cf_mod, rtnl_is_locked());
}

static int cgw_notifier(struct notifier_block *nb,
			unsigned long msg, void *ptr)
{
@@ -616,6 +629,7 @@ static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj, int type,
{
	struct rtcanmsg *rtcan;
	struct nlmsghdr *nlh;
	struct cf_mod *mod;

	nlh = nlmsg_put(skb, pid, seq, type, sizeof(*rtcan), flags);
	if (!nlh)
@@ -650,82 +664,83 @@ static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj, int type,
			goto cancel;
	}

	mod = cgw_job_cf_mod(gwj);
	if (gwj->flags & CGW_FLAGS_CAN_FD) {
		struct cgw_fdframe_mod mb;

		if (gwj->mod.modtype.and) {
			memcpy(&mb.cf, &gwj->mod.modframe.and, sizeof(mb.cf));
			mb.modtype = gwj->mod.modtype.and;
		if (mod->modtype.and) {
			memcpy(&mb.cf, &mod->modframe.and, sizeof(mb.cf));
			mb.modtype = mod->modtype.and;
			if (nla_put(skb, CGW_FDMOD_AND, sizeof(mb), &mb) < 0)
				goto cancel;
		}

		if (gwj->mod.modtype.or) {
			memcpy(&mb.cf, &gwj->mod.modframe.or, sizeof(mb.cf));
			mb.modtype = gwj->mod.modtype.or;
		if (mod->modtype.or) {
			memcpy(&mb.cf, &mod->modframe.or, sizeof(mb.cf));
			mb.modtype = mod->modtype.or;
			if (nla_put(skb, CGW_FDMOD_OR, sizeof(mb), &mb) < 0)
				goto cancel;
		}

		if (gwj->mod.modtype.xor) {
			memcpy(&mb.cf, &gwj->mod.modframe.xor, sizeof(mb.cf));
			mb.modtype = gwj->mod.modtype.xor;
		if (mod->modtype.xor) {
			memcpy(&mb.cf, &mod->modframe.xor, sizeof(mb.cf));
			mb.modtype = mod->modtype.xor;
			if (nla_put(skb, CGW_FDMOD_XOR, sizeof(mb), &mb) < 0)
				goto cancel;
		}

		if (gwj->mod.modtype.set) {
			memcpy(&mb.cf, &gwj->mod.modframe.set, sizeof(mb.cf));
			mb.modtype = gwj->mod.modtype.set;
		if (mod->modtype.set) {
			memcpy(&mb.cf, &mod->modframe.set, sizeof(mb.cf));
			mb.modtype = mod->modtype.set;
			if (nla_put(skb, CGW_FDMOD_SET, sizeof(mb), &mb) < 0)
				goto cancel;
		}
	} else {
		struct cgw_frame_mod mb;

		if (gwj->mod.modtype.and) {
			memcpy(&mb.cf, &gwj->mod.modframe.and, sizeof(mb.cf));
			mb.modtype = gwj->mod.modtype.and;
		if (mod->modtype.and) {
			memcpy(&mb.cf, &mod->modframe.and, sizeof(mb.cf));
			mb.modtype = mod->modtype.and;
			if (nla_put(skb, CGW_MOD_AND, sizeof(mb), &mb) < 0)
				goto cancel;
		}

		if (gwj->mod.modtype.or) {
			memcpy(&mb.cf, &gwj->mod.modframe.or, sizeof(mb.cf));
			mb.modtype = gwj->mod.modtype.or;
		if (mod->modtype.or) {
			memcpy(&mb.cf, &mod->modframe.or, sizeof(mb.cf));
			mb.modtype = mod->modtype.or;
			if (nla_put(skb, CGW_MOD_OR, sizeof(mb), &mb) < 0)
				goto cancel;
		}

		if (gwj->mod.modtype.xor) {
			memcpy(&mb.cf, &gwj->mod.modframe.xor, sizeof(mb.cf));
			mb.modtype = gwj->mod.modtype.xor;
		if (mod->modtype.xor) {
			memcpy(&mb.cf, &mod->modframe.xor, sizeof(mb.cf));
			mb.modtype = mod->modtype.xor;
			if (nla_put(skb, CGW_MOD_XOR, sizeof(mb), &mb) < 0)
				goto cancel;
		}

		if (gwj->mod.modtype.set) {
			memcpy(&mb.cf, &gwj->mod.modframe.set, sizeof(mb.cf));
			mb.modtype = gwj->mod.modtype.set;
		if (mod->modtype.set) {
			memcpy(&mb.cf, &mod->modframe.set, sizeof(mb.cf));
			mb.modtype = mod->modtype.set;
			if (nla_put(skb, CGW_MOD_SET, sizeof(mb), &mb) < 0)
				goto cancel;
		}
	}

	if (gwj->mod.uid) {
		if (nla_put_u32(skb, CGW_MOD_UID, gwj->mod.uid) < 0)
	if (mod->uid) {
		if (nla_put_u32(skb, CGW_MOD_UID, mod->uid) < 0)
			goto cancel;
	}

	if (gwj->mod.csumfunc.crc8) {
	if (mod->csumfunc.crc8) {
		if (nla_put(skb, CGW_CS_CRC8, CGW_CS_CRC8_LEN,
			    &gwj->mod.csum.crc8) < 0)
			    &mod->csum.crc8) < 0)
			goto cancel;
	}

	if (gwj->mod.csumfunc.xor) {
	if (mod->csumfunc.xor) {
		if (nla_put(skb, CGW_CS_XOR, CGW_CS_XOR_LEN,
			    &gwj->mod.csum.xor) < 0)
			    &mod->csum.xor) < 0)
			goto cancel;
	}

@@ -1059,7 +1074,7 @@ static int cgw_create_job(struct sk_buff *skb, struct nlmsghdr *nlh,
	struct net *net = sock_net(skb->sk);
	struct rtcanmsg *r;
	struct cgw_job *gwj;
	struct cf_mod mod;
	struct cf_mod *mod;
	struct can_can_gw ccgw;
	u8 limhops = 0;
	int err = 0;
@@ -1078,37 +1093,48 @@ static int cgw_create_job(struct sk_buff *skb, struct nlmsghdr *nlh,
	if (r->gwtype != CGW_TYPE_CAN_CAN)
		return -EINVAL;

	err = cgw_parse_attr(nlh, &mod, CGW_TYPE_CAN_CAN, &ccgw, &limhops);
	mod = kmalloc(sizeof(*mod), GFP_KERNEL);
	if (!mod)
		return -ENOMEM;

	err = cgw_parse_attr(nlh, mod, CGW_TYPE_CAN_CAN, &ccgw, &limhops);
	if (err < 0)
		return err;
		goto out_free_cf;

	if (mod.uid) {
	if (mod->uid) {
		ASSERT_RTNL();

		/* check for updating an existing job with identical uid */
		hlist_for_each_entry(gwj, &net->can.cgw_list, list) {
			if (gwj->mod.uid != mod.uid)
			struct cf_mod *old_cf;

			old_cf = cgw_job_cf_mod(gwj);
			if (old_cf->uid != mod->uid)
				continue;

			/* interfaces & filters must be identical */
			if (memcmp(&gwj->ccgw, &ccgw, sizeof(ccgw)))
				return -EINVAL;
			if (memcmp(&gwj->ccgw, &ccgw, sizeof(ccgw))) {
				err = -EINVAL;
				goto out_free_cf;
			}

			/* update modifications with disabled softirq & quit */
			local_bh_disable();
			memcpy(&gwj->mod, &mod, sizeof(mod));
			local_bh_enable();
			rcu_assign_pointer(gwj->cf_mod, mod);
			kfree_rcu_mightsleep(old_cf);
			return 0;
		}
	}

	/* ifindex == 0 is not allowed for job creation */
	if (!ccgw.src_idx || !ccgw.dst_idx)
		return -ENODEV;
	if (!ccgw.src_idx || !ccgw.dst_idx) {
		err = -ENODEV;
		goto out_free_cf;
	}

	gwj = kmem_cache_alloc(cgw_cache, GFP_KERNEL);
	if (!gwj)
		return -ENOMEM;
	if (!gwj) {
		err = -ENOMEM;
		goto out_free_cf;
	}

	gwj->handled_frames = 0;
	gwj->dropped_frames = 0;
@@ -1118,7 +1144,7 @@ static int cgw_create_job(struct sk_buff *skb, struct nlmsghdr *nlh,
	gwj->limit_hops = limhops;

	/* insert already parsed information */
	memcpy(&gwj->mod, &mod, sizeof(mod));
	RCU_INIT_POINTER(gwj->cf_mod, mod);
	memcpy(&gwj->ccgw, &ccgw, sizeof(ccgw));

	err = -ENODEV;
@@ -1152,9 +1178,11 @@ static int cgw_create_job(struct sk_buff *skb, struct nlmsghdr *nlh,
	if (!err)
		hlist_add_head_rcu(&gwj->list, &net->can.cgw_list);
out:
	if (err)
	if (err) {
		kmem_cache_free(cgw_cache, gwj);

out_free_cf:
		kfree(mod);
	}
	return err;
}

@@ -1214,19 +1242,22 @@ static int cgw_remove_job(struct sk_buff *skb, struct nlmsghdr *nlh,

	/* remove only the first matching entry */
	hlist_for_each_entry_safe(gwj, nx, &net->can.cgw_list, list) {
		struct cf_mod *cf_mod;

		if (gwj->flags != r->flags)
			continue;

		if (gwj->limit_hops != limhops)
			continue;

		cf_mod = cgw_job_cf_mod(gwj);
		/* we have a match when uid is enabled and identical */
		if (gwj->mod.uid || mod.uid) {
			if (gwj->mod.uid != mod.uid)
		if (cf_mod->uid || mod.uid) {
			if (cf_mod->uid != mod.uid)
				continue;
		} else {
			/* no uid => check for identical modifications */
			if (memcmp(&gwj->mod, &mod, sizeof(mod)))
			if (memcmp(cf_mod, &mod, sizeof(mod)))
				continue;
		}