Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756294AbaFRXTt (ORCPT ); Wed, 18 Jun 2014 19:19:49 -0400 Received: from mail-we0-f177.google.com ([74.125.82.177]:52505 "EHLO mail-we0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756142AbaFRXTr convert rfc822-to-8bit (ORCPT ); Wed, 18 Jun 2014 19:19:47 -0400 MIME-Version: 1.0 In-Reply-To: References: <20140618223457.GA31568@www.outflux.net> Date: Wed, 18 Jun 2014 16:19:45 -0700 Message-ID: Subject: Re: [PATCH] net: filter: fix upper BPF instruction limit From: Alexei Starovoitov To: Kees Cook 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 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… 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? -- 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/