All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleksij Rempel <o.rempel@pengutronix.de>
To: dev.kurt@vandijck-laurijssen.be, mkl@pengutronix.de, wg@grandegger.com
Cc: Oleksij Rempel <o.rempel@pengutronix.de>,
	syzbot+5322482fe520b02aea30@syzkaller.appspotmail.com,
	linux-stable <stable@vger.kernel.org>,
	kernel@pengutronix.de, linux-can@vger.kernel.org,
	netdev@vger.kernel.org, David Jander <david@protonic.nl>
Subject: [PATCH v1 2/5] can: j1939: transport: j1939_session_tx_dat(): fix use-after-free read in j1939_tp_txtimer()
Date: Fri,  7 Aug 2020 12:51:57 +0200	[thread overview]
Message-ID: <20200807105200.26441-3-o.rempel@pengutronix.de> (raw)
In-Reply-To: <20200807105200.26441-1-o.rempel@pengutronix.de>

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>
---
 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 b135c5e2a86e..30957c9a8eb7 100644
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -787,6 +787,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) {
@@ -1120,6 +1132,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;
-- 
2.28.0

  parent reply	other threads:[~2020-08-07 10:53 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-07 10:51 [PATCH v1 0/5] j1939 fixes Oleksij Rempel
2020-08-07 10:51 ` [PATCH v1 1/5] can: j1939: transport: j1939_simple_recv(): ignore local J1939 messages send not by J1939 stack Oleksij Rempel
2020-08-07 10:51 ` Oleksij Rempel [this message]
2020-08-07 10:51 ` [PATCH v1 3/5] can: j1939: socket: j1939_sk_bind(): make sure ml_priv is allocated Oleksij Rempel
2020-08-07 10:51 ` [PATCH v1 4/5] can: j1939: transport: add j1939_session_skb_find_by_offset() function Oleksij Rempel
2020-08-07 10:52 ` [PATCH v1 5/5] can: j1939: transport: j1939_xtp_rx_dat_one(): compare own packets to detect corruptions Oleksij Rempel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200807105200.26441-3-o.rempel@pengutronix.de \
    --to=o.rempel@pengutronix.de \
    --cc=david@protonic.nl \
    --cc=dev.kurt@vandijck-laurijssen.be \
    --cc=kernel@pengutronix.de \
    --cc=linux-can@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=syzbot+5322482fe520b02aea30@syzkaller.appspotmail.com \
    --cc=wg@grandegger.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.