Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d379b338 authored by Oliver Hartkopp's avatar Oliver Hartkopp Committed by Greg Kroah-Hartman
Browse files

can: gw: ensure DLC boundaries after CAN frame modification 



commit 0aaa81377c5a01f686bcdb8c7a6929a7bf330c68 upstream.

Muyu Yu provided a POC where user root with CAP_NET_ADMIN can create a CAN
frame modification rule that makes the data length code a higher value than
the available CAN frame data size. In combination with a configured checksum
calculation where the result is stored relatively to the end of the data
(e.g. cgw_csum_xor_rel) the tail of the skb (e.g. frag_list pointer in
skb_shared_info) can be rewritten which finally can cause a system crash.

Michael Kubecek suggested to drop frames that have a DLC exceeding the
available space after the modification process and provided a patch that can
handle CAN FD frames too. Within this patch we also limit the length for the
checksum calculations to the maximum of Classic CAN data length (8).

CAN frames that are dropped by these additional checks are counted with the
CGW_DELETED counter which indicates misconfigurations in can-gw rules.

This fixes CVE-2019-3701.

Reported-by: default avatarMuyu Yu <ieatmuttonchuan@gmail.com>
Reported-by: default avatarMarcus Meissner <meissner@suse.de>
Suggested-by: default avatarMichal Kubecek <mkubecek@suse.cz>
Tested-by: default avatarMuyu Yu <ieatmuttonchuan@gmail.com>
Tested-by: default avatarOliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: default avatarOliver Hartkopp <socketcan@hartkopp.net>
Cc: linux-stable <stable@vger.kernel.org> # >= v3.2
Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent a172c5d8

net/can/gw.c

+27 −3
Original line number Original line Diff line number Diff line
@@ -418,14 +418,30 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data) 
	while (modidx < MAX_MODFUNCTIONS && gwj->mod.modfunc[modidx])

	while (modidx < MAX_MODFUNCTIONS && gwj->mod.modfunc[modidx])

		(*gwj->mod.modfunc[modidx++])(cf, &gwj->mod);

		(*gwj->mod.modfunc[modidx++])(cf, &gwj->mod);





	/* check for checksum updates when the CAN frame has been modified */

	/* Has the CAN frame been modified? */

	if (modidx) {

	if (modidx) {

		if (gwj->mod.csumfunc.crc8)

		/* get available space for the processed CAN frame type */

		int max_len = nskb->len - offsetof(struct can_frame, data);



		/* dlc may have changed, make sure it fits to the CAN frame */

		if (cf->can_dlc > max_len)

			goto out_delete;



		/* check for checksum updates in classic CAN length only */

		if (gwj->mod.csumfunc.crc8) {

			if (cf->can_dlc > 8)

				goto out_delete;



			(*gwj->mod.csumfunc.crc8)(cf, &gwj->mod.csum.crc8);

			(*gwj->mod.csumfunc.crc8)(cf, &gwj->mod.csum.crc8);

		}



		if (gwj->mod.csumfunc.xor) {

			if (cf->can_dlc > 8)

				goto out_delete;





		if (gwj->mod.csumfunc.xor)

			(*gwj->mod.csumfunc.xor)(cf, &gwj->mod.csum.xor);

			(*gwj->mod.csumfunc.xor)(cf, &gwj->mod.csum.xor);

		}

		}

	}





	/* clear the skb timestamp if not configured the other way */

	/* clear the skb timestamp if not configured the other way */

	if (!(gwj->flags & CGW_FLAGS_CAN_SRC_TSTAMP))

	if (!(gwj->flags & CGW_FLAGS_CAN_SRC_TSTAMP))
@@ -436,6 +452,14 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data) 
		gwj->dropped_frames++;

		gwj->dropped_frames++;

	else

	else

		gwj->handled_frames++;

		gwj->handled_frames++;



	return;



 out_delete:

	/* delete frame due to misconfiguration */

	gwj->deleted_frames++;

	kfree_skb(nskb);

	return;

}

}





static inline int cgw_register_filter(struct cgw_job *gwj)

static inline int cgw_register_filter(struct cgw_job *gwj)