netdev - Re: [PATCH v1] j1939: transport: make sure EOMA is send with the total message size set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191025093626.snrkgseuuyjejvbv@pengutronix.de>
Date:   Fri, 25 Oct 2019 11:36:26 +0200
From:   Oleksij Rempel <o.rempel@...gutronix.de>
To:     dev.kurt@...dijck-laurijssen.be, mkl@...gutronix.de,
        wg@...ndegger.com
Cc:     kernel@...gutronix.de, linux-can@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH v1] j1939: transport: make sure EOMA is send with the
 total message size set

Hi all,

i decided to be strict here and drop malformed EOMA packages. At least
it makes testing easier.
I have no idea how far is it from reality. Will it brake some thing?


On Fri, Oct 25, 2019 at 11:30:15AM +0200, Oleksij Rempel wrote:
> We was sending malformed EOMA with total message size set to 0. So, fix this
> bug and add sanity check to the RX path.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@...gutronix.de>
> ---
>  net/can/j1939/transport.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
> index fe000ea757ea..a82097427642 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(session->priv->ndev, "%s: 0x%p: Incorrect size. Expected: %i; got: %i.\n",
> +			    __func__, session, session->total_message_size,
> +			    len);
> +		return;
> +	}
> +
>  	netdev_dbg(session->priv->ndev, "%s: 0x%p\n", __func__, session);
>  
>  	session->pkt.tx_acked = session->pkt.total;
> @@ -1432,7 +1450,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;
> -- 
> 2.23.0
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)