Skip to content

Commit

Permalink
Merge tag 'linux-can-fixes-for-6.8-20240214' of git://git.kernel.org/…
Browse files Browse the repository at this point in the history
…pub/scm/linux/kernel/git/mkl/linux-can

Marc Kleine-Budde says:

====================
pull-request: can 2024-02-14

this is a pull request of 3 patches for net/master.

the first patch is by Ziqi Zhao and targets the CAN J1939 protocol, it
fixes a potential deadlock by replacing the spinlock by an rwlock.

Oleksij Rempel's patch adds a missing spin_lock_bh() to prevent a
potential Use-After-Free in the CAN J1939's
setsockopt(SO_J1939_FILTER).

Maxime Jayat contributes a patch to fix the transceiver delay
compensation (TDCO) calculation, which is needed for higher CAN-FD bit
rates (usually 2Mbit/s).

* tag 'linux-can-fixes-for-6.8-20240214' of git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can:
  can: netlink: Fix TDCO calculation using the old data bittiming
  can: j1939: Fix UAF in j1939_sk_match_filter during setsockopt(SO_J1939_FILTER)
  can: j1939: prevent deadlock by changing j1939_socks_lock to rwlock
====================

Link: https://lore.kernel.org/r/20240214140348.2412776-1-mkl@pengutronix.de
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
  • Loading branch information
Paolo Abeni committed Feb 15, 2024
2 parents 488b6d9 + 2aa0a5e commit f3ac28e
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 19 deletions.
2 changes: 1 addition & 1 deletion drivers/net/can/dev/netlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
/* Neither of TDC parameters nor TDC flags are
* provided: do calculation
*/
can_calc_tdco(&priv->tdc, priv->tdc_const, &priv->data_bittiming,
can_calc_tdco(&priv->tdc, priv->tdc_const, &dbt,
&priv->ctrlmode, priv->ctrlmode_supported);
} /* else: both CAN_CTRLMODE_TDC_{AUTO,MANUAL} are explicitly
* turned off. TDC is disabled: do nothing
Expand Down
3 changes: 2 additions & 1 deletion net/can/j1939/j1939-priv.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ struct j1939_priv {
unsigned int tp_max_packet_size;

/* lock for j1939_socks list */
spinlock_t j1939_socks_lock;
rwlock_t j1939_socks_lock;
struct list_head j1939_socks;

struct kref rx_kref;
Expand Down Expand Up @@ -301,6 +301,7 @@ struct j1939_sock {

int ifindex;
struct j1939_addr addr;
spinlock_t filters_lock;
struct j1939_filter *filters;
int nfilters;
pgn_t pgn_rx_filter;
Expand Down
2 changes: 1 addition & 1 deletion net/can/j1939/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ struct j1939_priv *j1939_netdev_start(struct net_device *ndev)
return ERR_PTR(-ENOMEM);

j1939_tp_init(priv);
spin_lock_init(&priv->j1939_socks_lock);
rwlock_init(&priv->j1939_socks_lock);
INIT_LIST_HEAD(&priv->j1939_socks);

mutex_lock(&j1939_netdev_lock);
Expand Down
46 changes: 30 additions & 16 deletions net/can/j1939/socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,16 @@ static void j1939_jsk_add(struct j1939_priv *priv, struct j1939_sock *jsk)
jsk->state |= J1939_SOCK_BOUND;
j1939_priv_get(priv);

spin_lock_bh(&priv->j1939_socks_lock);
write_lock_bh(&priv->j1939_socks_lock);
list_add_tail(&jsk->list, &priv->j1939_socks);
spin_unlock_bh(&priv->j1939_socks_lock);
write_unlock_bh(&priv->j1939_socks_lock);
}

static void j1939_jsk_del(struct j1939_priv *priv, struct j1939_sock *jsk)
{
spin_lock_bh(&priv->j1939_socks_lock);
write_lock_bh(&priv->j1939_socks_lock);
list_del_init(&jsk->list);
spin_unlock_bh(&priv->j1939_socks_lock);
write_unlock_bh(&priv->j1939_socks_lock);

j1939_priv_put(priv);
jsk->state &= ~J1939_SOCK_BOUND;
Expand Down Expand Up @@ -262,12 +262,17 @@ static bool j1939_sk_match_dst(struct j1939_sock *jsk,
static bool j1939_sk_match_filter(struct j1939_sock *jsk,
const struct j1939_sk_buff_cb *skcb)
{
const struct j1939_filter *f = jsk->filters;
int nfilter = jsk->nfilters;
const struct j1939_filter *f;
int nfilter;

spin_lock_bh(&jsk->filters_lock);

f = jsk->filters;
nfilter = jsk->nfilters;

if (!nfilter)
/* receive all when no filters are assigned */
return true;
goto filter_match_found;

for (; nfilter; ++f, --nfilter) {
if ((skcb->addr.pgn & f->pgn_mask) != f->pgn)
Expand All @@ -276,9 +281,15 @@ static bool j1939_sk_match_filter(struct j1939_sock *jsk,
continue;
if ((skcb->addr.src_name & f->name_mask) != f->name)
continue;
return true;
goto filter_match_found;
}

spin_unlock_bh(&jsk->filters_lock);
return false;

filter_match_found:
spin_unlock_bh(&jsk->filters_lock);
return true;
}

static bool j1939_sk_recv_match_one(struct j1939_sock *jsk,
Expand Down Expand Up @@ -329,13 +340,13 @@ bool j1939_sk_recv_match(struct j1939_priv *priv, struct j1939_sk_buff_cb *skcb)
struct j1939_sock *jsk;
bool match = false;

spin_lock_bh(&priv->j1939_socks_lock);
read_lock_bh(&priv->j1939_socks_lock);
list_for_each_entry(jsk, &priv->j1939_socks, list) {
match = j1939_sk_recv_match_one(jsk, skcb);
if (match)
break;
}
spin_unlock_bh(&priv->j1939_socks_lock);
read_unlock_bh(&priv->j1939_socks_lock);

return match;
}
Expand All @@ -344,11 +355,11 @@ void j1939_sk_recv(struct j1939_priv *priv, struct sk_buff *skb)
{
struct j1939_sock *jsk;

spin_lock_bh(&priv->j1939_socks_lock);
read_lock_bh(&priv->j1939_socks_lock);
list_for_each_entry(jsk, &priv->j1939_socks, list) {
j1939_sk_recv_one(jsk, skb);
}
spin_unlock_bh(&priv->j1939_socks_lock);
read_unlock_bh(&priv->j1939_socks_lock);
}

static void j1939_sk_sock_destruct(struct sock *sk)
Expand Down Expand Up @@ -401,6 +412,7 @@ static int j1939_sk_init(struct sock *sk)
atomic_set(&jsk->skb_pending, 0);
spin_lock_init(&jsk->sk_session_queue_lock);
INIT_LIST_HEAD(&jsk->sk_session_queue);
spin_lock_init(&jsk->filters_lock);

/* j1939_sk_sock_destruct() depends on SOCK_RCU_FREE flag */
sock_set_flag(sk, SOCK_RCU_FREE);
Expand Down Expand Up @@ -703,9 +715,11 @@ static int j1939_sk_setsockopt(struct socket *sock, int level, int optname,
}

lock_sock(&jsk->sk);
spin_lock_bh(&jsk->filters_lock);
ofilters = jsk->filters;
jsk->filters = filters;
jsk->nfilters = count;
spin_unlock_bh(&jsk->filters_lock);
release_sock(&jsk->sk);
kfree(ofilters);
return 0;
Expand Down Expand Up @@ -1080,12 +1094,12 @@ void j1939_sk_errqueue(struct j1939_session *session,
}

/* spread RX notifications to all sockets subscribed to this session */
spin_lock_bh(&priv->j1939_socks_lock);
read_lock_bh(&priv->j1939_socks_lock);
list_for_each_entry(jsk, &priv->j1939_socks, list) {
if (j1939_sk_recv_match_one(jsk, &session->skcb))
__j1939_sk_errqueue(session, &jsk->sk, type);
}
spin_unlock_bh(&priv->j1939_socks_lock);
read_unlock_bh(&priv->j1939_socks_lock);
};

void j1939_sk_send_loop_abort(struct sock *sk, int err)
Expand Down Expand Up @@ -1273,15 +1287,15 @@ void j1939_sk_netdev_event_netdown(struct j1939_priv *priv)
struct j1939_sock *jsk;
int error_code = ENETDOWN;

spin_lock_bh(&priv->j1939_socks_lock);
read_lock_bh(&priv->j1939_socks_lock);
list_for_each_entry(jsk, &priv->j1939_socks, list) {
jsk->sk.sk_err = error_code;
if (!sock_flag(&jsk->sk, SOCK_DEAD))
sk_error_report(&jsk->sk);

j1939_sk_queue_drop_all(priv, jsk, error_code);
}
spin_unlock_bh(&priv->j1939_socks_lock);
read_unlock_bh(&priv->j1939_socks_lock);
}

static int j1939_sk_no_ioctlcmd(struct socket *sock, unsigned int cmd,
Expand Down

0 comments on commit f3ac28e

Please sign in to comment.