Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756389AbaFRX2r (ORCPT ); Wed, 18 Jun 2014 19:28:47 -0400 Received: from mail-oa0-f46.google.com ([209.85.219.46]:58648 "EHLO mail-oa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753602AbaFRX2q convert rfc822-to-8bit (ORCPT ); Wed, 18 Jun 2014 19:28:46 -0400 MIME-Version: 1.0 In-Reply-To: References: <20140618223457.GA31568@www.outflux.net> Date: Wed, 18 Jun 2014 16:28:45 -0700 X-Google-Sender-Auth: ZnmRrdts1jaPYmUPio2N3tuHcYs Message-ID: Subject: Re: [PATCH] net: filter: fix upper BPF instruction limit From: Kees Cook To: Alexei Starovoitov Cc: LKML , "David S. Miller" , Daniel Borkmann , Eric Dumazet , Chema Gonzalez , Network Development Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jun 18, 2014 at 4:19 PM, Alexei Starovoitov wrote: > On Wed, Jun 18, 2014 at 3:55 PM, Kees Cook wrote: >> On Wed, Jun 18, 2014 at 3:48 PM, Alexei Starovoitov wrote: >>> On Wed, Jun 18, 2014 at 3:34 PM, Kees Cook wrote: >>>> The original checks (via sk_chk_filter) for instruction count uses ">", >>>> not ">=", so changing this in sk_convert_filter has the potential to break >>>> existing seccomp filters that used exactly BPF_MAXINSNS many instructions. >>>> >>>> Fixes: bd4cf0ed331a ("net: filter: rework/optimize internal BPF interpreter's instruction set") >>>> Signed-off-by: Kees Cook >>>> Cc: stable@vger.kernel.org # v3.15+ >>> >>> Acked-by: Alexei Starovoitov >>> >>> I wonder how did you catch this? :) >>> Just code inspection or seccomp actually generating such programs? >> >> In the process of merging my seccomp thread-sync series back with >> mainline, I got uncomfortable that I was moving filter size validation >> around without actually testing it. When I added it, I was happy that >> my series was correctly checking size limits, but then discovered my >> newly added check actually failed on an earlier kernel (3.2). Tracking >> it down found the corner case under 3.15. >> >> Here's the test I added to the seccomp regression tests, if you're interested: >> https://github.com/kees/seccomp/commit/794d54a340cde70a3bdf7fe0ade1f95d160b2883 > > Nice. I'm assuming https://github.com/redpig/seccomp is still the main tree > for seccomp testsuite… Yes. Will hasn't pulled this most recent set of changes. > > btw I've tried to add 'real' test to it (one generated by chrome) > > +TEST(chrome_syscalls) { > + static struct sock_filter filter[] = { > + { 32, 240, 61, 4 }, /* 0: ld [4] */ > + { 21, 1, 0, -1073741762 }, /* 1: jeq #0xc000003e, 3, 2 */ > + { 5, 0, 0, 271 }, /* 2: ja 274 */ > + { 32, 208, 198, 0 }, /* 3: ld [0] */ > + { 69, 0, 1, 1073741824 }, /* 4: jset > #0x40000000, 5, 6 */ > + { 6, 0, 0, 196615 }, /* 5: ret #0x30007 */ > + { 53, 0, 7, 121 }, /* 6: jge #0x79, 7, 14 */ > + { 53, 0, 12, 214 }, /* 7: jge #0xd6, 8, 20 */ > … > + { 6, 0, 0, 2147418112 }, /* 272: ret #0x7fff0000 */ > + { 6, 0, 0, 327681 }, /* 273: ret #0x50001 */ > + { 6, 0, 0, 196610 }, /* 274: ret #0x30002 */ > + }; > ... > + for (i = 0; i < MAX_SYSCALLS; i++) { > + ch_pid = fork(); > + ASSERT_LE(0, ch_pid); > + > + if (ch_pid == 0) { > + ret = prctl(PR_SET_SECCOMP, > + SECCOMP_MODE_FILTER, &prog); > + ASSERT_EQ(0, ret); > +#define MAGIC (-1ll << 2) > + err = syscall(i, MAGIC, MAGIC, MAGIC, > + MAGIC, MAGIC, MAGIC); > + syscall(__NR_exit, 0); > + } > + wait(&status); > + if (status != expected_status[i]) > … > > but it's really x64 only and looks ugly. Do you have better ideas > on how to test all possible paths through auto-generated branch tree? For testing BPF application filter logic itself, I lean on the test suite in libseccomp, which does a ton of filter generation and testing. The seccomp regression tests in the github tree tend to focus on validating the basic behavioral elements (kill, trace, trap, etc). -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/