Commit e591d298 authored by David S. Miller's avatar David S. Miller

Merge tag 'linux-can-fixes-for-5.9-20200814' of...

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

Marc Kleine-Budde says:

====================
pull-request: can 2020-08-14

this is a pull request of 6 patches for net/master. All patches fix problems in
the j1939 CAN networking stack.

The first patch is by Eric Dumazet fixes a kernel-infoleak in
j1939_sk_sock2sockaddr_can().

The remaining 5 patches are by Oleksij Rempel and fix recption of j1939
messages not orginated by the stack, a use-after-free in j1939_tp_txtimer(),
ensure that the CAN driver has a ml_priv allocated. These problem were found by
google's syzbot. Further ETP sessions with block size of less than 255 are
fixed and a sanity check was added to j1939_xtp_rx_dat_one() to detect packet
corruption.
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 7fca4dee e052d054
...@@ -398,6 +398,7 @@ static int j1939_sk_init(struct sock *sk) ...@@ -398,6 +398,7 @@ static int j1939_sk_init(struct sock *sk)
spin_lock_init(&jsk->sk_session_queue_lock); spin_lock_init(&jsk->sk_session_queue_lock);
INIT_LIST_HEAD(&jsk->sk_session_queue); INIT_LIST_HEAD(&jsk->sk_session_queue);
sk->sk_destruct = j1939_sk_sock_destruct; sk->sk_destruct = j1939_sk_sock_destruct;
sk->sk_protocol = CAN_J1939;
return 0; return 0;
} }
...@@ -466,6 +467,14 @@ static int j1939_sk_bind(struct socket *sock, struct sockaddr *uaddr, int len) ...@@ -466,6 +467,14 @@ static int j1939_sk_bind(struct socket *sock, struct sockaddr *uaddr, int len)
goto out_release_sock; goto out_release_sock;
} }
if (!ndev->ml_priv) {
netdev_warn_once(ndev,
"No CAN mid layer private allocated, please fix your driver and use alloc_candev()!\n");
dev_put(ndev);
ret = -ENODEV;
goto out_release_sock;
}
priv = j1939_netdev_start(ndev); priv = j1939_netdev_start(ndev);
dev_put(ndev); dev_put(ndev);
if (IS_ERR(priv)) { if (IS_ERR(priv)) {
...@@ -553,6 +562,11 @@ static int j1939_sk_connect(struct socket *sock, struct sockaddr *uaddr, ...@@ -553,6 +562,11 @@ static int j1939_sk_connect(struct socket *sock, struct sockaddr *uaddr,
static void j1939_sk_sock2sockaddr_can(struct sockaddr_can *addr, static void j1939_sk_sock2sockaddr_can(struct sockaddr_can *addr,
const struct j1939_sock *jsk, int peer) const struct j1939_sock *jsk, int peer)
{ {
/* There are two holes (2 bytes and 3 bytes) to clear to avoid
* leaking kernel information to user space.
*/
memset(addr, 0, J1939_MIN_NAMELEN);
addr->can_family = AF_CAN; addr->can_family = AF_CAN;
addr->can_ifindex = jsk->ifindex; addr->can_ifindex = jsk->ifindex;
addr->can_addr.j1939.pgn = jsk->addr.pgn; addr->can_addr.j1939.pgn = jsk->addr.pgn;
......
...@@ -352,17 +352,16 @@ void j1939_session_skb_queue(struct j1939_session *session, ...@@ -352,17 +352,16 @@ void j1939_session_skb_queue(struct j1939_session *session,
skb_queue_tail(&session->skb_queue, skb); skb_queue_tail(&session->skb_queue, skb);
} }
static struct sk_buff *j1939_session_skb_find(struct j1939_session *session) static struct
sk_buff *j1939_session_skb_find_by_offset(struct j1939_session *session,
unsigned int offset_start)
{ {
struct j1939_priv *priv = session->priv; struct j1939_priv *priv = session->priv;
struct j1939_sk_buff_cb *do_skcb;
struct sk_buff *skb = NULL; struct sk_buff *skb = NULL;
struct sk_buff *do_skb; struct sk_buff *do_skb;
struct j1939_sk_buff_cb *do_skcb;
unsigned int offset_start;
unsigned long flags; unsigned long flags;
offset_start = session->pkt.dpo * 7;
spin_lock_irqsave(&session->skb_queue.lock, flags); spin_lock_irqsave(&session->skb_queue.lock, flags);
skb_queue_walk(&session->skb_queue, do_skb) { skb_queue_walk(&session->skb_queue, do_skb) {
do_skcb = j1939_skb_to_cb(do_skb); do_skcb = j1939_skb_to_cb(do_skb);
...@@ -382,6 +381,14 @@ static struct sk_buff *j1939_session_skb_find(struct j1939_session *session) ...@@ -382,6 +381,14 @@ static struct sk_buff *j1939_session_skb_find(struct j1939_session *session)
return skb; return skb;
} }
static struct sk_buff *j1939_session_skb_find(struct j1939_session *session)
{
unsigned int offset_start;
offset_start = session->pkt.dpo * 7;
return j1939_session_skb_find_by_offset(session, offset_start);
}
/* see if we are receiver /* see if we are receiver
* returns 0 for broadcasts, although we will receive them * returns 0 for broadcasts, although we will receive them
*/ */
...@@ -766,7 +773,7 @@ static int j1939_session_tx_dat(struct j1939_session *session) ...@@ -766,7 +773,7 @@ static int j1939_session_tx_dat(struct j1939_session *session)
int ret = 0; int ret = 0;
u8 dat[8]; u8 dat[8];
se_skb = j1939_session_skb_find(session); se_skb = j1939_session_skb_find_by_offset(session, session->pkt.tx * 7);
if (!se_skb) if (!se_skb)
return -ENOBUFS; return -ENOBUFS;
...@@ -787,6 +794,18 @@ static int j1939_session_tx_dat(struct j1939_session *session) ...@@ -787,6 +794,18 @@ static int j1939_session_tx_dat(struct j1939_session *session)
if (len > 7) if (len > 7)
len = 7; len = 7;
if (offset + len > se_skb->len) {
netdev_err_once(priv->ndev,
"%s: 0x%p: requested data outside of queued buffer: offset %i, len %i, pkt.tx: %i\n",
__func__, session, skcb->offset, se_skb->len , session->pkt.tx);
return -EOVERFLOW;
}
if (!len) {
ret = -ENOBUFS;
break;
}
memcpy(&dat[1], &tpdat[offset], len); memcpy(&dat[1], &tpdat[offset], len);
ret = j1939_tp_tx_dat(session, dat, len + 1); ret = j1939_tp_tx_dat(session, dat, len + 1);
if (ret < 0) { if (ret < 0) {
...@@ -1120,6 +1139,9 @@ static enum hrtimer_restart j1939_tp_txtimer(struct hrtimer *hrtimer) ...@@ -1120,6 +1139,9 @@ static enum hrtimer_restart j1939_tp_txtimer(struct hrtimer *hrtimer)
* cleanup including propagation of the error to user space. * cleanup including propagation of the error to user space.
*/ */
break; break;
case -EOVERFLOW:
j1939_session_cancel(session, J1939_XTP_ABORT_ECTS_TOO_BIG);
break;
case 0: case 0:
session->tx_retry = 0; session->tx_retry = 0;
break; break;
...@@ -1750,7 +1772,8 @@ static void j1939_xtp_rx_dat_one(struct j1939_session *session, ...@@ -1750,7 +1772,8 @@ static void j1939_xtp_rx_dat_one(struct j1939_session *session,
__func__, session); __func__, session);
goto out_session_cancel; goto out_session_cancel;
} }
se_skb = j1939_session_skb_find(session);
se_skb = j1939_session_skb_find_by_offset(session, packet * 7);
if (!se_skb) { if (!se_skb) {
netdev_warn(priv->ndev, "%s: 0x%p: no skb found\n", __func__, netdev_warn(priv->ndev, "%s: 0x%p: no skb found\n", __func__,
session); session);
...@@ -1769,7 +1792,20 @@ static void j1939_xtp_rx_dat_one(struct j1939_session *session, ...@@ -1769,7 +1792,20 @@ static void j1939_xtp_rx_dat_one(struct j1939_session *session,
} }
tpdat = se_skb->data; tpdat = se_skb->data;
memcpy(&tpdat[offset], &dat[1], nbytes); if (!session->transmission) {
memcpy(&tpdat[offset], &dat[1], nbytes);
} else {
int err;
err = memcmp(&tpdat[offset], &dat[1], nbytes);
if (err)
netdev_err_once(priv->ndev,
"%s: 0x%p: Data of RX-looped back packet (%*ph) doesn't match TX data (%*ph)!\n",
__func__, session,
nbytes, &dat[1],
nbytes, &tpdat[offset]);
}
if (packet == session->pkt.rx) if (packet == session->pkt.rx)
session->pkt.rx++; session->pkt.rx++;
...@@ -2017,6 +2053,10 @@ void j1939_simple_recv(struct j1939_priv *priv, struct sk_buff *skb) ...@@ -2017,6 +2053,10 @@ void j1939_simple_recv(struct j1939_priv *priv, struct sk_buff *skb)
if (!skb->sk) if (!skb->sk)
return; return;
if (skb->sk->sk_family != AF_CAN ||
skb->sk->sk_protocol != CAN_J1939)
return;
j1939_session_list_lock(priv); j1939_session_list_lock(priv);
session = j1939_session_get_simple(priv, skb); session = j1939_session_get_simple(priv, skb);
j1939_session_list_unlock(priv); j1939_session_list_unlock(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