Oleksij Rempel (3): can: j1939: transport: add j1939_session_skb_find_by_offset() function can: j1939: transport: j1939_session_tx_dat(): fix use-after-free read in j1939_tp_txtimer() can: j1939: fix Use-after-Free, hold skb ref while in use
net/can/j1939/transport.c | 77 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 63 insertions(+), 14 deletions(-)
反馈： 您发送到kernel@openeuler.org的补丁/补丁集，已成功转换为PR！ PR链接地址： https://gitee.com/openeuler/kernel/pulls/9179 邮件列表地址：https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/3...
FeedBack: The patch(es) which you have sent to kernel@openeuler.org mailing list has been converted to a pull request successfully! Pull request link: https://gitee.com/openeuler/kernel/pulls/9179 Mailing list address: https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/3...
From: Oleksij Rempel o.rempel@pengutronix.de
mainline inclusion from mainline-v5.9-rc2 commit 840835c9281215341d84966a8855f267a971e6a3 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I9R4CE CVE: CVE-2021-47232
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
Sometimes it makes no sense to search the skb by pkt.dpo, since we need next the skb within the transaction block. This may happen if we have an ETP session with CTS set to less than 255 packets.
After this patch, we will be able to work with ETP sessions where the block size (ETP.CM_CTS byte 2) is less than 255 packets.
Reported-by: Henrique Figueira henrislip@gmail.com Reported-by: https://github.com/linux-can/can-utils/issues/228 Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol") Signed-off-by: Oleksij Rempel o.rempel@pengutronix.de Link: https://lore.kernel.org/r/20200807105200.26441-5-o.rempel@pengutronix.de Signed-off-by: Marc Kleine-Budde mkl@pengutronix.de Signed-off-by: Zhang Changzhong zhangchangzhong@huawei.com --- net/can/j1939/transport.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c index 9f99af5..3320d70 100644 --- a/net/can/j1939/transport.c +++ b/net/can/j1939/transport.c @@ -352,17 +352,16 @@ void j1939_session_skb_queue(struct j1939_session *session, 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_sk_buff_cb *do_skcb; struct sk_buff *skb = NULL; struct sk_buff *do_skb; - struct j1939_sk_buff_cb *do_skcb; - unsigned int offset_start; unsigned long flags;
- offset_start = session->pkt.dpo * 7; - spin_lock_irqsave(&session->skb_queue.lock, flags); skb_queue_walk(&session->skb_queue, 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) 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 * returns 0 for broadcasts, although we will receive them */ @@ -766,7 +773,7 @@ static int j1939_session_tx_dat(struct j1939_session *session) int ret = 0; 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) return -ENOBUFS;
@@ -1750,7 +1757,8 @@ static void j1939_xtp_rx_dat_one(struct j1939_session *session, __func__, session); 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) { netdev_warn(priv->ndev, "%s: 0x%p: no skb found\n", __func__, session);
From: Oleksij Rempel o.rempel@pengutronix.de
mainline inclusion from mainline-v5.9-rc2 commit cd3b3636c99fcac52c598b64061f3fe4413c6a12 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I9R4CE CVE: CVE-2021-47232
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
The current stack implementation do not support ECTS requests of not aligned TP sized blocks.
If ECTS will request a block with size and offset spanning two TP blocks, this will cause memcpy() to read beyond the queued skb (which does only contain one TP sized block).
Sometimes KASAN will detect this read if the memory region beyond the skb was previously allocated and freed. In other situations it will stay undetected. The ETP transfer in any case will be corrupted.
This patch adds a sanity check to avoid this kind of read and abort the session with error J1939_XTP_ABORT_ECTS_TOO_BIG.
Reported-by: syzbot+5322482fe520b02aea30@syzkaller.appspotmail.com Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol") Cc: linux-stable stable@vger.kernel.org # >= v5.4 Signed-off-by: Oleksij Rempel o.rempel@pengutronix.de Link: https://lore.kernel.org/r/20200807105200.26441-3-o.rempel@pengutronix.de Signed-off-by: Marc Kleine-Budde mkl@pengutronix.de Signed-off-by: Zhang Changzhong zhangchangzhong@huawei.com --- net/can/j1939/transport.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c index 3320d70..566fcf9 100644 --- a/net/can/j1939/transport.c +++ b/net/can/j1939/transport.c @@ -794,6 +794,18 @@ static int j1939_session_tx_dat(struct j1939_session *session) if (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); ret = j1939_tp_tx_dat(session, dat, len + 1); if (ret < 0) { @@ -1127,6 +1139,9 @@ static enum hrtimer_restart j1939_tp_txtimer(struct hrtimer *hrtimer) * cleanup including propagation of the error to user space. */ break; + case -EOVERFLOW: + j1939_session_cancel(session, J1939_XTP_ABORT_ECTS_TOO_BIG); + break; case 0: session->tx_retry = 0; break;
From: Oleksij Rempel o.rempel@pengutronix.de
mainline inclusion from mainline-v5.13-rc7 commit 2030043e616cab40f510299f09b636285e0a3678 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I9R4CE CVE: CVE-2021-47232
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
This patch fixes a Use-after-Free found by the syzbot.
The problem is that a skb is taken from the per-session skb queue, without incrementing the ref count. This leads to a Use-after-Free if the skb is taken concurrently from the session queue due to a CTS.
Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol") Link: https://lore.kernel.org/r/20210521115720.7533-1-o.rempel@pengutronix.de Cc: Hillf Danton hdanton@sina.com Cc: linux-stable stable@vger.kernel.org Reported-by: syzbot+220c1a29987a9a490903@syzkaller.appspotmail.com Reported-by: syzbot+45199c1b73b4013525cf@syzkaller.appspotmail.com Signed-off-by: Oleksij Rempel o.rempel@pengutronix.de Signed-off-by: Marc Kleine-Budde mkl@pengutronix.de Signed-off-by: Zhang Changzhong zhangchangzhong@huawei.com --- net/can/j1939/transport.c | 54 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 14 deletions(-)
diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c index 566fcf9..1a68b00 100644 --- a/net/can/j1939/transport.c +++ b/net/can/j1939/transport.c @@ -330,6 +330,9 @@ static void j1939_session_skb_drop_old(struct j1939_session *session)
if ((do_skcb->offset + do_skb->len) < offset_start) { __skb_unlink(do_skb, &session->skb_queue); + /* drop ref taken in j1939_session_skb_queue() */ + skb_unref(do_skb); + kfree_skb(do_skb); } spin_unlock_irqrestore(&session->skb_queue.lock, flags); @@ -349,12 +352,13 @@ void j1939_session_skb_queue(struct j1939_session *session,
skcb->flags |= J1939_ECU_LOCAL_SRC;
+ skb_get(skb); skb_queue_tail(&session->skb_queue, skb); }
static struct -sk_buff *j1939_session_skb_find_by_offset(struct j1939_session *session, - unsigned int offset_start) +sk_buff *j1939_session_skb_get_by_offset(struct j1939_session *session, + unsigned int offset_start) { struct j1939_priv *priv = session->priv; struct j1939_sk_buff_cb *do_skcb; @@ -371,6 +375,10 @@ sk_buff *j1939_session_skb_find_by_offset(struct j1939_session *session, skb = do_skb; } } + + if (skb) + skb_get(skb); + spin_unlock_irqrestore(&session->skb_queue.lock, flags);
if (!skb) @@ -381,12 +389,12 @@ sk_buff *j1939_session_skb_find_by_offset(struct j1939_session *session, return skb; }
-static struct sk_buff *j1939_session_skb_find(struct j1939_session *session) +static struct sk_buff *j1939_session_skb_get(struct j1939_session *session) { unsigned int offset_start;
offset_start = session->pkt.dpo * 7; - return j1939_session_skb_find_by_offset(session, offset_start); + return j1939_session_skb_get_by_offset(session, offset_start); }
/* see if we are receiver @@ -773,7 +781,7 @@ static int j1939_session_tx_dat(struct j1939_session *session) int ret = 0; u8 dat[8];
- se_skb = j1939_session_skb_find_by_offset(session, session->pkt.tx * 7); + se_skb = j1939_session_skb_get_by_offset(session, session->pkt.tx * 7); if (!se_skb) return -ENOBUFS;
@@ -798,7 +806,8 @@ static int j1939_session_tx_dat(struct j1939_session *session) 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; + ret = -EOVERFLOW; + goto out_free; }
if (!len) { @@ -832,6 +841,12 @@ static int j1939_session_tx_dat(struct j1939_session *session) if (pkt_done) j1939_tp_set_rxtimeout(session, 250);
+ out_free: + if (ret) + kfree_skb(se_skb); + else + consume_skb(se_skb); + return ret; }
@@ -1004,7 +1019,7 @@ static int j1939_xtp_txnext_receiver(struct j1939_session *session) static int j1939_simple_txnext(struct j1939_session *session) { struct j1939_priv *priv = session->priv; - struct sk_buff *se_skb = j1939_session_skb_find(session); + struct sk_buff *se_skb = j1939_session_skb_get(session); struct sk_buff *skb; int ret;
@@ -1012,8 +1027,10 @@ static int j1939_simple_txnext(struct j1939_session *session) return 0;
skb = skb_clone(se_skb, GFP_ATOMIC); - if (!skb) - return -ENOMEM; + if (!skb) { + ret = -ENOMEM; + goto out_free; + }
can_skb_set_owner(skb, se_skb->sk);
@@ -1021,12 +1038,18 @@ static int j1939_simple_txnext(struct j1939_session *session)
ret = j1939_send_one(priv, skb); if (ret) - return ret; + goto out_free;
j1939_sk_errqueue(session, J1939_ERRQUEUE_SCHED); j1939_sk_queue_activate_next(session);
- return 0; + out_free: + if (ret) + kfree_skb(se_skb); + else + consume_skb(se_skb); + + return ret; }
static bool j1939_session_deactivate_locked(struct j1939_session *session) @@ -1167,9 +1190,10 @@ static void j1939_session_completed(struct j1939_session *session) struct sk_buff *skb;
if (!session->transmission) { - skb = j1939_session_skb_find(session); + skb = j1939_session_skb_get(session); /* distribute among j1939 receivers */ j1939_sk_recv(session->priv, skb); + consume_skb(skb); }
j1939_session_deactivate_activate_next(session); @@ -1732,7 +1756,7 @@ static void j1939_xtp_rx_dat_one(struct j1939_session *session, { struct j1939_priv *priv = session->priv; struct j1939_sk_buff_cb *skcb; - struct sk_buff *se_skb; + struct sk_buff *se_skb = NULL; const u8 *dat; u8 *tpdat; int offset; @@ -1773,7 +1797,7 @@ static void j1939_xtp_rx_dat_one(struct j1939_session *session, goto out_session_cancel; }
- se_skb = j1939_session_skb_find_by_offset(session, packet * 7); + se_skb = j1939_session_skb_get_by_offset(session, packet * 7); if (!se_skb) { netdev_warn(priv->ndev, "%s: 0x%p: no skb found\n", __func__, session); @@ -1816,11 +1840,13 @@ static void j1939_xtp_rx_dat_one(struct j1939_session *session, j1939_tp_set_rxtimeout(session, 250); } session->last_cmd = 0xff; + consume_skb(se_skb); j1939_session_put(session);
return;
out_session_cancel: + kfree_skb(se_skb); j1939_session_timers_cancel(session); j1939_session_cancel(session, J1939_XTP_ABORT_FAULT); j1939_session_put(session);
-
patchwork bot -
Zhang Changzhong