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: 9d71dd0c70 ("can: add support of SAE J1939 protocol")
Suggested-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
Tested-by: Oleksij Rempel <o.rempel@pengutronix.de>
Acked-by: Oleksij 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: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
db15e90a8c
commit
8a46c4a2bc
1 changed files with 11 additions and 11 deletions
|
|
@ -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)
|
||||||
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)
|
||||||
{
|
{
|
||||||
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)
|
||||||
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)
|
||||||
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)
|
||||||
|
|
||||||
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);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue