Skip to content

Commit cd3b363

Browse files
oleremmarckleinebudde
authored andcommitted
can: j1939: transport: j1939_session_tx_dat(): fix use-after-free read in j1939_tp_txtimer()
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: [email protected] Fixes: 9d71dd0 ("can: add support of SAE J1939 protocol") Cc: linux-stable <[email protected]> # >= v5.4 Signed-off-by: Oleksij Rempel <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Marc Kleine-Budde <[email protected]>
1 parent b43e3a8 commit cd3b363

File tree

1 file changed

+15
-0
lines changed

1 file changed

+15
-0
lines changed

net/can/j1939/transport.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,18 @@ static int j1939_session_tx_dat(struct j1939_session *session)
787787
if (len > 7)
788788
len = 7;
789789

790+
if (offset + len > se_skb->len) {
791+
netdev_err_once(priv->ndev,
792+
"%s: 0x%p: requested data outside of queued buffer: offset %i, len %i, pkt.tx: %i\n",
793+
__func__, session, skcb->offset, se_skb->len , session->pkt.tx);
794+
return -EOVERFLOW;
795+
}
796+
797+
if (!len) {
798+
ret = -ENOBUFS;
799+
break;
800+
}
801+
790802
memcpy(&dat[1], &tpdat[offset], len);
791803
ret = j1939_tp_tx_dat(session, dat, len + 1);
792804
if (ret < 0) {
@@ -1120,6 +1132,9 @@ static enum hrtimer_restart j1939_tp_txtimer(struct hrtimer *hrtimer)
11201132
* cleanup including propagation of the error to user space.
11211133
*/
11221134
break;
1135+
case -EOVERFLOW:
1136+
j1939_session_cancel(session, J1939_XTP_ABORT_ECTS_TOO_BIG);
1137+
break;
11231138
case 0:
11241139
session->tx_retry = 0;
11251140
break;

0 commit comments

Comments
 (0)