From patchwork Mon Sep 11 17:56:01 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gregory Rose X-Patchwork-Id: 812521 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="Ddtbgt8i"; dkim-atps=neutral Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3xrbKf4yzjz9s7F for ; Tue, 12 Sep 2017 03:59:34 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id A1129B56; Mon, 11 Sep 2017 17:56:29 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 48C25B30 for ; Mon, 11 Sep 2017 17:56:26 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pf0-f196.google.com (mail-pf0-f196.google.com [209.85.192.196]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id BC96AA4 for ; Mon, 11 Sep 2017 17:56:25 +0000 (UTC) Received: by mail-pf0-f196.google.com with SMTP id i23so401123pfi.2 for ; Mon, 11 Sep 2017 10:56:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=VyC5nfaIBsQFXfPun5s8FCA08bl6efNI8b/Cc/PCfkY=; b=Ddtbgt8iCeiq/KMbxxqjPL7vw45VNxWPRl5GDDvWR5r2mWbgS2FFK8pc3vwGOu1H4Y PLq62aqzbdx4D2kz9DvilRMKpyfbi5jsR+CaSDNGozer6Xu7PvE52w44gBbo0xudXeqt PaKSB/tBUMgfJlJ98K49hi8p3BbCWixOuVs0ezoRFbcL6YU0gz24HUcpwqkltcfkad+h +6N0ZNpvOV4ZRh2MM7VjZdAxbm9FPKt2W7lG955ZdQ9jzWsCb6OFW3HtM/CofYsZyAte BvRDq7YbC4qUEXgjfEGvjCaz7MsRwJ0uO+dOlJ50ZOsWZwBtz+T4U2oH3oEUTpzzSI+N YoTg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=VyC5nfaIBsQFXfPun5s8FCA08bl6efNI8b/Cc/PCfkY=; b=UcEnwZ48A1hDhqHGkABx+1LBEkzxZyEOwcMTSUqXjh9PNxMFPL8b3fD/rQz6BNdkja 5Slxb9a4NotIleGxMU8ZxN0Ql1tM5q/xtw7Sjt7Ro+C5//7H34oIhQdmyOdRuiuezBDa LJa37xiTrQUJCMTG+2DUNdIXIs+1v/iflHYxILYl1YuOgxz+u8arW8usB1OJNUNZwiHM x1pvnmm79pc3wRKPT1Pr/uC+u2Eim1l4H+cNasI/3gnAt8TF4chJbgzoq0Z2yQyEGqAi CFlJ+ZQXNFZgucZUZcbbXy4MSsg4e6BVY81xubnxYQ2UzIFPF5bhVZMlNxCfe69TEpEc pGiw== X-Gm-Message-State: AHPjjUjzQ8YhKLaFHhUN1cF/bSKb1dz33SwVg3P9FaWmIlqEoUDLZWOk dMwtCgod9z9My7W8 X-Google-Smtp-Source: ADKCNb7iE6JrJ7GEOwFcTR0wxhMaybhpNoQITKg2CfYXGD2voeRVvef6B1neTvXJBf7PZbnzGdnVIA== X-Received: by 10.98.236.17 with SMTP id k17mr11802579pfh.191.1505152584891; Mon, 11 Sep 2017 10:56:24 -0700 (PDT) Received: from gizo.domain (67-5-132-83.ptld.qwest.net. [67.5.132.83]) by smtp.gmail.com with ESMTPSA id h185sm16733084pfe.160.2017.09.11.10.56.23 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 11 Sep 2017 10:56:23 -0700 (PDT) From: Greg Rose To: dev@openvswitch.org Date: Mon, 11 Sep 2017 10:56:01 -0700 Message-Id: <1505152570-6143-6-git-send-email-gvrose8192@gmail.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1505152570-6143-1-git-send-email-gvrose8192@gmail.com> References: <1505152570-6143-1-git-send-email-gvrose8192@gmail.com> X-Spam-Status: No, score=0.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=disabled version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH 06/15] datapath: fix skb_panic due to the incorrect actions attrlen X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org Upstream commit: commit 494bea39f3201776cdfddc232705f54a0bd210c4 Author: Liping Zhang Date: Wed Aug 16 13:30:07 2017 +0800 openvswitch: fix skb_panic due to the incorrect actions attrlen For sw_flow_actions, the actions_len only represents the kernel part's size, and when we dump the actions to the userspace, we will do the convertions, so it's true size may become bigger than the actions_len. But unfortunately, for OVS_PACKET_ATTR_ACTIONS, we use the actions_len to alloc the skbuff, so the user_skb's size may become insufficient and oops will happen like this: skbuff: skb_over_panic: text:ffffffff8148fabf len:1749 put:157 head: ffff881300f39000 data:ffff881300f39000 tail:0x6d5 end:0x6c0 dev: ------------[ cut here ]------------ kernel BUG at net/core/skbuff.c:129! [...] Call Trace: [] skb_put+0x43/0x44 [] skb_zerocopy+0x6c/0x1f4 [] queue_userspace_packet+0x3a3/0x448 [openvswitch] [] ovs_dp_upcall+0x30/0x5c [openvswitch] [] output_userspace+0x132/0x158 [openvswitch] [] ? ip6_rcv_finish+0x74/0x77 [ipv6] [] do_execute_actions+0xcc1/0xdc8 [openvswitch] [] ovs_execute_actions+0x74/0x106 [openvswitch] [] ovs_dp_process_packet+0xe1/0xfd [openvswitch] [] ? key_extract+0x63c/0x8d5 [openvswitch] [] ovs_vport_receive+0xa1/0xc3 [openvswitch] [...] Also we can find that the actions_len is much little than the orig_len: crash> struct sw_flow_actions 0xffff8812f539d000 struct sw_flow_actions { rcu = { next = 0xffff8812f5398800, func = 0xffffe3b00035db32 }, orig_len = 1384, actions_len = 592, actions = 0xffff8812f539d01c } So as a quick fix, use the orig_len instead of the actions_len to alloc the user_skb. Last, this oops happened on our system running a relative old kernel, but the same risk still exists on the mainline, since we use the wrong actions_len from the beginning. Fixes: ccea74457bbd ("openvswitch: include datapath actions with sampled-pac Cc: Neil McKee Signed-off-by: Liping Zhang Acked-by: Pravin B Shelar Signed-off-by: David S. Miller Fixes: 0e469d3b380c ("datapath: Include datapath actions with sampled-packet upcall to userspace.") Signed-off-by: Greg Rose --- datapath/actions.c | 1 + datapath/datapath.c | 7 ++++--- datapath/datapath.h | 2 ++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/datapath/actions.c b/datapath/actions.c index 59d91b2..ad18c2c 100644 --- a/datapath/actions.c +++ b/datapath/actions.c @@ -1348,6 +1348,7 @@ int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb, goto out; } + OVS_CB(skb)->acts_origlen = acts->orig_len; err = do_execute_actions(dp, skb, key, acts->actions, acts->actions_len); diff --git a/datapath/datapath.c b/datapath/datapath.c index b565fc5..1780819 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -388,7 +388,7 @@ static int queue_gso_packets(struct datapath *dp, struct sk_buff *skb, } static size_t upcall_msg_size(const struct dp_upcall_info *upcall_info, - unsigned int hdrlen) + unsigned int hdrlen, int actions_attrlen) { size_t size = NLMSG_ALIGN(sizeof(struct ovs_header)) + nla_total_size(hdrlen) /* OVS_PACKET_ATTR_PACKET */ @@ -405,7 +405,7 @@ static size_t upcall_msg_size(const struct dp_upcall_info *upcall_info, /* OVS_PACKET_ATTR_ACTIONS */ if (upcall_info->actions_len) - size += nla_total_size(upcall_info->actions_len); + size += nla_total_size(actions_attrlen); /* OVS_PACKET_ATTR_MRU */ if (upcall_info->mru) @@ -472,7 +472,8 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb, else hlen = skb->len; - len = upcall_msg_size(upcall_info, hlen - cutlen); + len = upcall_msg_size(upcall_info, hlen - cutlen, + OVS_CB(skb)->acts_origlen); user_skb = genlmsg_new(len, GFP_ATOMIC); if (!user_skb) { err = -ENOMEM; diff --git a/datapath/datapath.h b/datapath/datapath.h index f20deed..70ad0ac 100644 --- a/datapath/datapath.h +++ b/datapath/datapath.h @@ -100,11 +100,13 @@ struct datapath { * when a packet is received by OVS. * @mru: The maximum received fragement size; 0 if the packet is not * fragmented. + * @acts_origlen: The netlink size of the flow actions applied to this skb. * @cutlen: The number of bytes from the packet end to be removed. */ struct ovs_skb_cb { struct vport *input_vport; u16 mru; + u16 acts_origlen; u32 cutlen; }; #define OVS_CB(skb) ((struct ovs_skb_cb *)(skb)->cb)