Commit cd9c790d authored by Fedor Pchelkin's avatar Fedor Pchelkin Committed by Marc Kleine-Budde

can: j1939: change j1939_netdev_lock type to mutex

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>
parent 2a84aea8
...@@ -126,7 +126,7 @@ static void j1939_can_recv(struct sk_buff *iskb, void *data) ...@@ -126,7 +126,7 @@ static void j1939_can_recv(struct sk_buff *iskb, void *data)
#define J1939_CAN_ID CAN_EFF_FLAG #define J1939_CAN_ID CAN_EFF_FLAG
#define J1939_CAN_MASK (CAN_EFF_FLAG | CAN_RTR_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) static struct j1939_priv *j1939_priv_create(struct net_device *ndev)
{ {
...@@ -220,7 +220,7 @@ static void __j1939_rx_release(struct kref *kref) ...@@ -220,7 +220,7 @@ static void __j1939_rx_release(struct kref *kref)
j1939_can_rx_unregister(priv); j1939_can_rx_unregister(priv);
j1939_ecu_unmap_all(priv); j1939_ecu_unmap_all(priv);
j1939_priv_set(priv->ndev, NULL); j1939_priv_set(priv->ndev, NULL);
spin_unlock(&j1939_netdev_lock); mutex_unlock(&j1939_netdev_lock);
} }
/* get pointer to priv without increasing ref counter */ /* get pointer to priv without increasing ref counter */
...@@ -248,9 +248,9 @@ static struct j1939_priv *j1939_priv_get_by_ndev(struct net_device *ndev) ...@@ -248,9 +248,9 @@ static struct j1939_priv *j1939_priv_get_by_ndev(struct net_device *ndev)
{ {
struct j1939_priv *priv; struct j1939_priv *priv;
spin_lock(&j1939_netdev_lock); mutex_lock(&j1939_netdev_lock);
priv = j1939_priv_get_by_ndev_locked(ndev); priv = j1939_priv_get_by_ndev_locked(ndev);
spin_unlock(&j1939_netdev_lock); mutex_unlock(&j1939_netdev_lock);
return priv; return priv;
} }
...@@ -260,14 +260,14 @@ struct j1939_priv *j1939_netdev_start(struct net_device *ndev) ...@@ -260,14 +260,14 @@ struct j1939_priv *j1939_netdev_start(struct net_device *ndev)
struct j1939_priv *priv, *priv_new; struct j1939_priv *priv, *priv_new;
int ret; int ret;
spin_lock(&j1939_netdev_lock); mutex_lock(&j1939_netdev_lock);
priv = j1939_priv_get_by_ndev_locked(ndev); priv = j1939_priv_get_by_ndev_locked(ndev);
if (priv) { if (priv) {
kref_get(&priv->rx_kref); kref_get(&priv->rx_kref);
spin_unlock(&j1939_netdev_lock); mutex_unlock(&j1939_netdev_lock);
return priv; return priv;
} }
spin_unlock(&j1939_netdev_lock); mutex_unlock(&j1939_netdev_lock);
priv = j1939_priv_create(ndev); priv = j1939_priv_create(ndev);
if (!priv) if (!priv)
...@@ -277,20 +277,20 @@ struct j1939_priv *j1939_netdev_start(struct net_device *ndev) ...@@ -277,20 +277,20 @@ struct j1939_priv *j1939_netdev_start(struct net_device *ndev)
spin_lock_init(&priv->j1939_socks_lock); spin_lock_init(&priv->j1939_socks_lock);
INIT_LIST_HEAD(&priv->j1939_socks); 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); priv_new = j1939_priv_get_by_ndev_locked(ndev);
if (priv_new) { if (priv_new) {
/* Someone was faster than us, use their priv and roll /* Someone was faster than us, use their priv and roll
* back our's. * back our's.
*/ */
kref_get(&priv_new->rx_kref); kref_get(&priv_new->rx_kref);
spin_unlock(&j1939_netdev_lock); mutex_unlock(&j1939_netdev_lock);
dev_put(ndev); dev_put(ndev);
kfree(priv); kfree(priv);
return priv_new; return priv_new;
} }
j1939_priv_set(ndev, priv); j1939_priv_set(ndev, priv);
spin_unlock(&j1939_netdev_lock); mutex_unlock(&j1939_netdev_lock);
ret = j1939_can_rx_register(priv); ret = j1939_can_rx_register(priv);
if (ret < 0) if (ret < 0)
...@@ -308,7 +308,7 @@ struct j1939_priv *j1939_netdev_start(struct net_device *ndev) ...@@ -308,7 +308,7 @@ struct j1939_priv *j1939_netdev_start(struct net_device *ndev)
void j1939_netdev_stop(struct j1939_priv *priv) 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); j1939_priv_put(priv);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment