From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexei Starovoitov Subject: Re: [PATCH] net: filter: fix upper BPF instruction limit Date: Wed, 18 Jun 2014 16:19:45 -0700 Message-ID: References: <20140618223457.GA31568@www.outflux.net> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: LKML , "David S. Miller" , Daniel Borkmann , Eric Dumazet , Chema Gonzalez , Network Development To: Kees Cook Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Wed, Jun 18, 2014 at 3:55 PM, Kees Cook wrot= e: > On Wed, Jun 18, 2014 at 3:48 PM, Alexei Starovoitov wrote: >> On Wed, Jun 18, 2014 at 3:34 PM, Kees Cook w= rote: >>> The original checks (via sk_chk_filter) for instruction count uses = ">", >>> not ">=3D", so changing this in sk_convert_filter has the potential= to break >>> existing seccomp filters that used exactly BPF_MAXINSNS many instru= ctions. >>> >>> Fixes: bd4cf0ed331a ("net: filter: rework/optimize internal BPF int= erpreter'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 validatio= n > 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). Trackin= g > it down found the corner case under 3.15. > > Here's the test I added to the seccomp regression tests, if you're in= terested: > https://github.com/kees/seccomp/commit/794d54a340cde70a3bdf7fe0ade1f9= 5d160b2883 Nice. I'm assuming https://github.com/redpig/seccomp is still the main = tree for seccomp testsuite=E2=80=A6 btw I've tried to add 'real' test to it (one generated by chrome) +TEST(chrome_syscalls) { + static struct sock_filter filter[] =3D { + { 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, 1= 4 */ + { 53, 0, 12, 214 }, /* 7: jge #0xd6, 8, 2= 0 */ =E2=80=A6 + { 6, 0, 0, 2147418112 }, /* 272: ret #0x7fff0000= */ + { 6, 0, 0, 327681 }, /* 273: ret #0x50001 */ + { 6, 0, 0, 196610 }, /* 274: ret #0x30002 */ + }; =2E.. + for (i =3D 0; i < MAX_SYSCALLS; i++) { + ch_pid =3D fork(); + ASSERT_LE(0, ch_pid); + + if (ch_pid =3D=3D 0) { + ret =3D prctl(PR_SET_SECCOMP, + SECCOMP_MODE_FILTER, &prog); + ASSERT_EQ(0, ret); +#define MAGIC (-1ll << 2) + err =3D syscall(i, MAGIC, MAGIC, MAGIC, + MAGIC, MAGIC, MAGIC); + syscall(__NR_exit, 0); + } + wait(&status); + if (status !=3D expected_status[i]) =E2=80=A6 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?