Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit bf0245bd authored by Fedor Pchelkin's avatar Fedor Pchelkin Committed by Greg Kroah-Hartman
Browse files

can: j1939: change j1939_netdev_lock type to mutex 



commit cd9c790de2088b0d797dc4d244b4f174f9962554 upstream.

It turns out access to j1939_can_rx_register() needs to be serialized,
otherwise j1939_priv can be corrupted when parallel threads call
j1939_netdev_start() and j1939_can_rx_register() fails. This issue is
thoroughly covered in other commit which serializes access to
j1939_can_rx_register().

Change j1939_netdev_lock type to mutex so that we do not need to remove
GFP_KERNEL from can_rx_register().

j1939_netdev_lock seems to be used in normal contexts where mutex usage
is not prohibited.

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

Fixes: 9d71dd0c ("can: add support of SAE J1939 protocol")
Suggested-by: default avatarAlexey Khoroshilov <khoroshilov@ispras.ru>
Signed-off-by: default avatarFedor Pchelkin <pchelkin@ispras.ru>
Tested-by: default avatarOleksij Rempel <o.rempel@pengutronix.de>
Acked-by: default avatarOleksij Rempel <o.rempel@pengutronix.de>
Link: https://lore.kernel.org/r/20230526171910.227615-2-pchelkin@ispras.ru


Cc: stable@vger.kernel.org
Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 9eed68d6

net/can/j1939/main.c

+11 −11
Original line number Diff line number Diff line
@@ -122,7 +122,7 @@ static void j1939_can_recv(struct sk_buff *iskb, void *data) 
#define J1939_CAN_ID CAN_EFF_FLAG

#define J1939_CAN_MASK (CAN_EFF_FLAG | CAN_RTR_FLAG)



static DEFINE_SPINLOCK(j1939_netdev_lock);

static DEFINE_MUTEX(j1939_netdev_lock);



static struct j1939_priv *j1939_priv_create(struct net_device *ndev)

{
@@ -216,7 +216,7 @@ static void __j1939_rx_release(struct kref *kref) 
	j1939_can_rx_unregister(priv);

	j1939_ecu_unmap_all(priv);

	j1939_priv_set(priv->ndev, NULL);

	spin_unlock(&j1939_netdev_lock);

	mutex_unlock(&j1939_netdev_lock);

}



/* get pointer to priv without increasing ref counter */
@@ -244,9 +244,9 @@ static struct j1939_priv *j1939_priv_get_by_ndev(struct net_device *ndev) 
{

	struct j1939_priv *priv;



	spin_lock(&j1939_netdev_lock);

	mutex_lock(&j1939_netdev_lock);

	priv = j1939_priv_get_by_ndev_locked(ndev);

	spin_unlock(&j1939_netdev_lock);

	mutex_unlock(&j1939_netdev_lock);



	return priv;

}
@@ -256,14 +256,14 @@ struct j1939_priv *j1939_netdev_start(struct net_device *ndev) 
	struct j1939_priv *priv, *priv_new;

	int ret;



	spin_lock(&j1939_netdev_lock);

	mutex_lock(&j1939_netdev_lock);

	priv = j1939_priv_get_by_ndev_locked(ndev);

	if (priv) {

		kref_get(&priv->rx_kref);

		spin_unlock(&j1939_netdev_lock);

		mutex_unlock(&j1939_netdev_lock);

		return priv;

	}

	spin_unlock(&j1939_netdev_lock);

	mutex_unlock(&j1939_netdev_lock);



	priv = j1939_priv_create(ndev);

	if (!priv)
@@ -273,20 +273,20 @@ struct j1939_priv *j1939_netdev_start(struct net_device *ndev) 
	spin_lock_init(&priv->j1939_socks_lock);

	INIT_LIST_HEAD(&priv->j1939_socks);



	spin_lock(&j1939_netdev_lock);

	mutex_lock(&j1939_netdev_lock);

	priv_new = j1939_priv_get_by_ndev_locked(ndev);

	if (priv_new) {

		/* Someone was faster than us, use their priv and roll

		 * back our's.

		 */

		kref_get(&priv_new->rx_kref);

		spin_unlock(&j1939_netdev_lock);

		mutex_unlock(&j1939_netdev_lock);

		dev_put(ndev);

		kfree(priv);

		return priv_new;

	}

	j1939_priv_set(ndev, priv);

	spin_unlock(&j1939_netdev_lock);

	mutex_unlock(&j1939_netdev_lock);



	ret = j1939_can_rx_register(priv);

	if (ret < 0)
@@ -304,7 +304,7 @@ struct j1939_priv *j1939_netdev_start(struct net_device *ndev) 


void j1939_netdev_stop(struct j1939_priv *priv)

{

	kref_put_lock(&priv->rx_kref, __j1939_rx_release, &j1939_netdev_lock);

	kref_put_mutex(&priv->rx_kref, __j1939_rx_release, &j1939_netdev_lock);

	j1939_priv_put(priv);

}