From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DCE6CC4360F for ; Thu, 4 Apr 2019 11:00:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 93636206BA for ; Thu, 4 Apr 2019 11:00:34 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="GiKzQgFX" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728909AbfDDLAd (ORCPT ); Thu, 4 Apr 2019 07:00:33 -0400 Received: from mail-yw1-f68.google.com ([209.85.161.68]:34717 "EHLO mail-yw1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728645AbfDDLAc (ORCPT ); Thu, 4 Apr 2019 07:00:32 -0400 Received: by mail-yw1-f68.google.com with SMTP id x129so800944ywc.1 for ; Thu, 04 Apr 2019 04:00:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ZV10f4XzxcIIVR0rnbrDo2vM6lQXTWkSPs4fbMF8G50=; b=GiKzQgFXymx8z3l7Z3cyDc38eWLksVAHgUP4vw3jhqxli5TcDq7GeQ9ZGXc4WS+iSv 3DTt6skpqqdzrWJM5RZ4BK/RSi0vBWJvYcp3tNJ96z+NGpOX3q28L114CQxOuAhD8xse 94MeFYIy1XBkWPGrpeJQDt5SweVMHZiRdli8W7GpXfvCFf7Mn3t2Knm9S3ZagJyktyqz vPSe+vhh02yEK6XEGjkpTTrMONpf7nKxXTWE5mu7xR4WkZx0cPw+ZECswcPxXh48LRkY oiYVbL+77sYUTwI8pld85HtjXkhidvTUxPsd7peTWf16Nf8zmumD2xZS2Ns0vs+m1632 X5iA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ZV10f4XzxcIIVR0rnbrDo2vM6lQXTWkSPs4fbMF8G50=; b=MQNz5eoG+JC2By5O6Rm3DXpPKwp4+E3oLqT+U698aQN4+c4iEfCR0sS2p3i6e3uS9V wBNkdTDoG3WXg1KMvNfQb96jYTjqfrS/7+9geCJidiSdlLiz13RQxYlwWLWi2RUHUWmG DfBXvMhtDTstBC7mKlwJAKdnMYI/utTADDTdDgoIjDY1ql5wDgUlIpo1YsU2fBodMspJ udc0+aaHBFAQhN5Hs9510AJspKDn9rbWycKzfPrObI1A+XcBFmOi3wctGcFejMhGXkCn /gVOe5SBgD47TeL6XIVba590RP2Fs7QZr2kSezqGF9dc0jScPwvLxDzpU9ff4pt70ISI RbJA== X-Gm-Message-State: APjAAAWmfwwnDt9V62Pnt9FhcKpDczGGrFQEHwN0adso8v+Lwu2MduPR Y40ZQNHu2pseVRI/YZDVAhHLmKBGM22/U2/s8zQ= X-Google-Smtp-Source: APXvYqy8x5YnAoPQM8ni0bJUlnUKl0kukdgsYi0wDX2aYnAh7tgArIwOvHn34OLMq+1Ha9FHDp8ghV+y4wKqQ1qkcs4= X-Received: by 2002:a81:350d:: with SMTP id c13mr4113537ywa.242.1554375631528; Thu, 04 Apr 2019 04:00:31 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: yotam gigi Date: Thu, 4 Apr 2019 14:00:20 +0300 Message-ID: Subject: Re: [PATCH net] net/sched: act_sample: fix divide by zero in the traffic path To: Davide Caratti Cc: mcroce@redhat.com, netdev@vger.kernel.org, Jamal Hadi Salim , Cong Wang , Jiri Pirko , "David S. Miller" , Yotam Gigi Content-Type: text/plain; charset="UTF-8" Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu, Apr 4, 2019 at 1:32 PM Davide Caratti wrote: > > the control path of 'sample' action does not validate the value of 'rate' > provided by the user, but then it uses it as divisor in the traffic path. > Validate it in tcf_sample_init(), and return -EINVAL with a proper extack > message in case that value is zero, to fix a splat with the script below: > > # tc f a dev test0 egress matchall action sample rate 0 group 1 index 2 > # tc -s a s action sample > total acts 1 > > action order 0: sample rate 1/0 group 1 pipe > index 2 ref 1 bind 1 installed 19 sec used 19 sec > Action statistics: > Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) > backlog 0b 0p requeues 0 > # ping 192.0.2.1 -I test0 -c1 -q > > divide error: 0000 [#1] SMP PTI > CPU: 1 PID: 6192 Comm: ping Not tainted 5.1.0-rc2.diag2+ #591 > Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011 > RIP: 0010:tcf_sample_act+0x9e/0x1e0 [act_sample] > Code: 6a f1 85 c0 74 0d 80 3d 83 1a 00 00 00 0f 84 9c 00 00 00 4d 85 e4 0f 84 85 00 00 00 e8 9b d7 9c f1 44 8b 8b e0 00 00 00 31 d2 <41> f7 f1 85 d2 75 70 f6 85 83 00 00 00 10 48 8b 45 10 8b 88 08 01 > RSP: 0018:ffffae320190ba30 EFLAGS: 00010246 > RAX: 00000000b0677d21 RBX: ffff8af1ed9ec000 RCX: 0000000059a9fe49 > RDX: 0000000000000000 RSI: 000000000c7e33b7 RDI: ffff8af23daa0af0 > RBP: ffff8af1ee11b200 R08: 0000000074fcaf7e R09: 0000000000000000 > R10: 0000000000000050 R11: ffffffffb3088680 R12: ffff8af232307f80 > R13: 0000000000000003 R14: ffff8af1ed9ec000 R15: 0000000000000000 > FS: 00007fe9c6d2f740(0000) GS:ffff8af23da80000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00007fff6772f000 CR3: 00000000746a2004 CR4: 00000000001606e0 > Call Trace: > tcf_action_exec+0x7c/0x1c0 > tcf_classify+0x57/0x160 > __dev_queue_xmit+0x3dc/0xd10 > ip_finish_output2+0x257/0x6d0 > ip_output+0x75/0x280 > ip_send_skb+0x15/0x40 > raw_sendmsg+0xae3/0x1410 > sock_sendmsg+0x36/0x40 > __sys_sendto+0x10e/0x140 > __x64_sys_sendto+0x24/0x30 > do_syscall_64+0x60/0x210 > entry_SYSCALL_64_after_hwframe+0x49/0xbe > [...] > Kernel panic - not syncing: Fatal exception in interrupt > > Add a TDC selftest to document that 'rate' is now being validated. > > Reported-by: Matteo Croce > Fixes: 5c5670fae430 ("net/sched: Introduce sample tc action") > Signed-off-by: Davide Caratti Acked-by: Yotam Gigi Thanks! > --- > net/sched/act_sample.c | 10 ++++++-- > .../tc-testing/tc-tests/actions/sample.json | 24 +++++++++++++++++++ > 2 files changed, 32 insertions(+), 2 deletions(-) > > diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c > index 4060b0955c97..0f82d50ea232 100644 > --- a/net/sched/act_sample.c > +++ b/net/sched/act_sample.c > @@ -45,8 +45,8 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla, > struct nlattr *tb[TCA_SAMPLE_MAX + 1]; > struct psample_group *psample_group; > struct tcf_chain *goto_ch = NULL; > + u32 psample_group_num, rate; > struct tc_sample *parm; > - u32 psample_group_num; > struct tcf_sample *s; > bool exists = false; > int ret, err; > @@ -85,6 +85,12 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla, > if (err < 0) > goto release_idr; > > + rate = nla_get_u32(tb[TCA_SAMPLE_RATE]); > + if (!rate) { > + NL_SET_ERR_MSG(extack, "invalid sample rate"); > + err = -EINVAL; > + goto put_chain; > + } > psample_group_num = nla_get_u32(tb[TCA_SAMPLE_PSAMPLE_GROUP]); > psample_group = psample_group_get(net, psample_group_num); > if (!psample_group) { > @@ -96,7 +102,7 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla, > > spin_lock_bh(&s->tcf_lock); > goto_ch = tcf_action_set_ctrlact(*a, parm->action, goto_ch); > - s->rate = nla_get_u32(tb[TCA_SAMPLE_RATE]); > + s->rate = rate; > s->psample_group_num = psample_group_num; > RCU_INIT_POINTER(s->psample_group, psample_group); > > diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/sample.json b/tools/testing/selftests/tc-testing/tc-tests/actions/sample.json > index 27f0acaed880..ddabb160a11b 100644 > --- a/tools/testing/selftests/tc-testing/tc-tests/actions/sample.json > +++ b/tools/testing/selftests/tc-testing/tc-tests/actions/sample.json > @@ -143,6 +143,30 @@ > "$TC actions flush action sample" > ] > }, > + { > + "id": "7571", > + "name": "Add sample action with invalid rate", > + "category": [ > + "actions", > + "sample" > + ], > + "setup": [ > + [ > + "$TC actions flush action sample", > + 0, > + 1, > + 255 > + ] > + ], > + "cmdUnderTest": "$TC actions add action sample rate 0 group 1 index 2", > + "expExitCode": "255", > + "verifyCmd": "$TC actions get action sample index 2", > + "matchPattern": "action order [0-9]+: sample rate 1/0 group 1.*index 2 ref", > + "matchCount": "0", > + "teardown": [ > + "$TC actions flush action sample" > + ] > + }, > { > "id": "b6d4", > "name": "Add sample action with mandatory arguments and invalid control action", > -- > 2.20.1 >