From db1a804cca6fe0cea9dea888d50dda134713c340 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Wed, 18 Sep 2019 11:11:56 +0100 Subject: can: j1939: fix resource leak of skb on error return paths Currently the error return paths do not free skb and this results in a memory leak. Fix this by freeing them before the return. Addresses-Coverity: ("Resource leak") Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol") Signed-off-by: Colin Ian King Acked-by: Oleksij Rempel Signed-off-by: Marc Kleine-Budde --- net/can/j1939/socket.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'net') diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c index 37c1040bcb9c..5c6eabcb5df1 100644 --- a/net/can/j1939/socket.c +++ b/net/can/j1939/socket.c @@ -909,8 +909,10 @@ void j1939_sk_errqueue(struct j1939_session *session, memset(serr, 0, sizeof(*serr)); switch (type) { case J1939_ERRQUEUE_ACK: - if (!(sk->sk_tsflags & SOF_TIMESTAMPING_TX_ACK)) + if (!(sk->sk_tsflags & SOF_TIMESTAMPING_TX_ACK)) { + kfree_skb(skb); return; + } serr->ee.ee_errno = ENOMSG; serr->ee.ee_origin = SO_EE_ORIGIN_TIMESTAMPING; @@ -918,8 +920,10 @@ void j1939_sk_errqueue(struct j1939_session *session, state = "ACK"; break; case J1939_ERRQUEUE_SCHED: - if (!(sk->sk_tsflags & SOF_TIMESTAMPING_TX_SCHED)) + if (!(sk->sk_tsflags & SOF_TIMESTAMPING_TX_SCHED)) { + kfree_skb(skb); return; + } serr->ee.ee_errno = ENOMSG; serr->ee.ee_origin = SO_EE_ORIGIN_TIMESTAMPING; -- cgit v1.2.3-59-g8ed1b From 896daf723c845289a4ea1e68e74a5a5475aa796d Mon Sep 17 00:00:00 2001 From: Oleksij Rempel Date: Thu, 10 Oct 2019 12:50:31 +0200 Subject: can: j1939: fix memory leak if filters was set Filters array is coped from user space and linked to the j1939 socket. On socket release this memory was not freed. Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol") Signed-off-by: Oleksij Rempel Signed-off-by: Marc Kleine-Budde --- net/can/j1939/socket.c | 1 + 1 file changed, 1 insertion(+) (limited to 'net') diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c index 5c6eabcb5df1..4d8ba701e15d 100644 --- a/net/can/j1939/socket.c +++ b/net/can/j1939/socket.c @@ -580,6 +580,7 @@ static int j1939_sk_release(struct socket *sock) j1939_netdev_stop(priv); } + kfree(jsk->filters); sock_orphan(sk); sock->sk = NULL; -- cgit v1.2.3-59-g8ed1b From eaa654f164ba9acd5656e6485eeb5e73da8bfc3e Mon Sep 17 00:00:00 2001 From: Oleksij Rempel Date: Fri, 25 Oct 2019 15:04:13 +0200 Subject: can: j1939: transport: j1939_session_fresh_new(): make sure EOMA is send with the total message size set We were sending malformed EOMA messageswith total message size set to 0. This patch fixes the bug. Reported-by: https://github.com/linux-can/can-utils/issues/159 Signed-off-by: Oleksij Rempel Acked-by: Kurt Van Dijck Signed-off-by: Marc Kleine-Budde --- net/can/j1939/transport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net') diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c index fe000ea757ea..06183d6f4fb7 100644 --- a/net/can/j1939/transport.c +++ b/net/can/j1939/transport.c @@ -1432,7 +1432,7 @@ j1939_session *j1939_session_fresh_new(struct j1939_priv *priv, skcb = j1939_skb_to_cb(skb); memcpy(skcb, rel_skcb, sizeof(*skcb)); - session = j1939_session_new(priv, skb, skb->len); + session = j1939_session_new(priv, skb, size); if (!session) { kfree_skb(skb); return NULL; -- cgit v1.2.3-59-g8ed1b From 688d11c38423fd98ab6c8d5fc1976c8f365fc875 Mon Sep 17 00:00:00 2001 From: Oleksij Rempel Date: Fri, 25 Oct 2019 15:04:13 +0200 Subject: can: j1939: transport: j1939_xtp_rx_eoma_one(): Add sanity check for correct total message size We were sending malformed EOMA with total message size set to 0. This issue has been fixed in the previous patch. In this patch a sanity check is added to the RX path and a error message is displayed. Signed-off-by: Oleksij Rempel Signed-off-by: Marc Kleine-Budde --- net/can/j1939/transport.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'net') diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c index 06183d6f4fb7..e5f1a56994c6 100644 --- a/net/can/j1939/transport.c +++ b/net/can/j1939/transport.c @@ -1273,9 +1273,27 @@ j1939_xtp_rx_abort(struct j1939_priv *priv, struct sk_buff *skb, static void j1939_xtp_rx_eoma_one(struct j1939_session *session, struct sk_buff *skb) { + struct j1939_sk_buff_cb *skcb = j1939_skb_to_cb(skb); + const u8 *dat; + int len; + if (j1939_xtp_rx_cmd_bad_pgn(session, skb)) return; + dat = skb->data; + + if (skcb->addr.type == J1939_ETP) + len = j1939_etp_ctl_to_size(dat); + else + len = j1939_tp_ctl_to_size(dat); + + if (session->total_message_size != len) { + netdev_warn_once(session->priv->ndev, + "%s: 0x%p: Incorrect size. Expected: %i; got: %i.\n", + __func__, session, session->total_message_size, + len); + } + netdev_dbg(session->priv->ndev, "%s: 0x%p\n", __func__, session); session->pkt.tx_acked = session->pkt.total; -- cgit v1.2.3-59-g8ed1b