From patchwork Thu Jan 10 21:02:53 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oliver Hartkopp X-Patchwork-Id: 1023219 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=hartkopp.net Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=hartkopp.net header.i=@hartkopp.net header.b="C0bQdGlK"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 43bJPD21N3z9sDL for ; Fri, 11 Jan 2019 08:03:12 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730971AbfAJVDK (ORCPT ); Thu, 10 Jan 2019 16:03:10 -0500 Received: from mo4-p01-ob.smtp.rzone.de ([85.215.255.51]:15186 "EHLO mo4-p01-ob.smtp.rzone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726164AbfAJVDK (ORCPT ); Thu, 10 Jan 2019 16:03:10 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1547154186; s=strato-dkim-0002; d=hartkopp.net; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: X-RZG-CLASS-ID:X-RZG-AUTH:From:Subject:Sender; bh=abmjyowXF7mxE5zYxNsO/UPGy9XA1LyQdboaCbnySTM=; b=C0bQdGlKnFGW1naLX2N5PS+11EydcLAR3wQRAk/KMmjMo4+R5uTkEso25cWSqErZWS L24OwFcXsoweFg2eiFOw6HIek0rZaudpLG6Vl4bX8QIebyaGYLvvf70zs8K1RTL2mnxl +mAF9h0nC0DeD5zuGPGjPUNLXFas6ceqRaR+BocRJ+9UQW6oq5CUgiCd9u/UUCe+t9ku 6DbWgqHWBRQGDmIrCfOQSCkYhQVbhST7sTITiOjxPeTnklp0imRtkbUym8zhm437Xwfd xDXUTLHaYAETP9oYS1lYNlsuo9iUkGVQHF3bG+ga0l4wtu2LyOYqUomBZDwXo20m9jLn jVYw== X-RZG-AUTH: ":P2MHfkW8eP4Mre39l357AZT/I7AY/7nT2yrDxb8mjGrp7owjzFK3JbFk1mS1k+8Az01boHV61Be+aQ==" X-RZG-CLASS-ID: mo00 Received: from zbook.lan by smtp.strato.de (RZmta 44.9 DYNA|AUTH) with ESMTPSA id j01e49v0AL33QzT (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (curve secp521r1 with 521 ECDH bits, eq. 15360 bits RSA)) (Client did not present a certificate); Thu, 10 Jan 2019 22:03:03 +0100 (CET) From: Oliver Hartkopp To: linux-can@vger.kernel.org Cc: ieatmuttonchuan@gmail.com, meissner@suse.de, mkubecek@suse.cz, netdev@vger.kernel.org Subject: [PATCH 1/3] can: gw: ensure DLC boundaries after CAN frame modification Date: Thu, 10 Jan 2019 22:02:53 +0100 Message-Id: <20190110210255.4291-2-socketcan@hartkopp.net> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190110210255.4291-1-socketcan@hartkopp.net> References: <20190110210255.4291-1-socketcan@hartkopp.net> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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: Muyu Yu Reported-by: Marcus Meissner Suggested-by: Michal Kubecek Tested-by: Muyu Yu Tested-by: Oliver Hartkopp Signed-off-by: Oliver Hartkopp Cc: linux-stable # >= v3.2 Signed-off-by: Marc Kleine-Budde --- net/can/gw.c | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/net/can/gw.c b/net/can/gw.c index faa3da88a127..53859346dc9a 100644 --- a/net/can/gw.c +++ b/net/can/gw.c @@ -416,13 +416,29 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data) while (modidx < MAX_MODFUNCTIONS && gwj->mod.modfunc[modidx]) (*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 (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); + } + + 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); + } } /* clear the skb timestamp if not configured the other way */ @@ -434,6 +450,14 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data) gwj->dropped_frames++; else 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 net *net, struct cgw_job *gwj)