Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751931AbdHHBED (ORCPT ); Mon, 7 Aug 2017 21:04:03 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:34825 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751719AbdHHBEB (ORCPT ); Mon, 7 Aug 2017 21:04:01 -0400 From: Tyler Hicks Subject: Re: [PATCH 1/4] seccomp: Provide matching filter for introspection To: Kees Cook , linux-kernel@vger.kernel.org Cc: Fabricio Voznika , Andy Lutomirski , Will Drewry , Shuah Khan , linux-kselftest@vger.kernel.org, linux-security-module@vger.kernel.org References: <1501730353-46840-1-git-send-email-keescook@chromium.org> <1501730353-46840-2-git-send-email-keescook@chromium.org> Message-ID: Date: Mon, 7 Aug 2017 20:03:47 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <1501730353-46840-2-git-send-email-keescook@chromium.org> Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="agWPSeJBgenvDUmdhS9mG3B4VweSiOhFa" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5370 Lines: 140 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --agWPSeJBgenvDUmdhS9mG3B4VweSiOhFa Content-Type: multipart/mixed; boundary="op9NH7wawAvILpcjR1kSRQmI5w77C4IlQ"; protected-headers="v1" From: Tyler Hicks To: Kees Cook , linux-kernel@vger.kernel.org Cc: Fabricio Voznika , Andy Lutomirski , Will Drewry , Shuah Khan , linux-kselftest@vger.kernel.org, linux-security-module@vger.kernel.org Message-ID: Subject: Re: [PATCH 1/4] seccomp: Provide matching filter for introspection References: <1501730353-46840-1-git-send-email-keescook@chromium.org> <1501730353-46840-2-git-send-email-keescook@chromium.org> In-Reply-To: <1501730353-46840-2-git-send-email-keescook@chromium.org> --op9NH7wawAvILpcjR1kSRQmI5w77C4IlQ Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 08/02/2017 10:19 PM, Kees Cook wrote: > Both the upcoming logging improvements and changes to RET_KILL will nee= d > to know which filter a given seccomp return value originated from. In > order to delay logic processing of result until after the seccomp loop,= > this adds a single pointer assignment on matches. This will allow both > log and RET_KILL logic to work off the filter rather than doing more > expensive tests inside the time-critical run_filters loop. >=20 > Running tight cycles of getpid() with filters attached shows no measura= ble > difference in speed. >=20 > Suggested-by: Tyler Hicks > Signed-off-by: Kees Cook > --- > kernel/seccomp.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) >=20 > diff --git a/kernel/seccomp.c b/kernel/seccomp.c > index 98b59b5db90b..8bdcf01379e4 100644 > --- a/kernel/seccomp.c > +++ b/kernel/seccomp.c > @@ -171,10 +171,12 @@ static int seccomp_check_filter(struct sock_filte= r *filter, unsigned int flen) > /** > * seccomp_run_filters - evaluates all seccomp filters against @sd > * @sd: optional seccomp data to be passed to filters > + * @match: stores struct seccomp_filter that resulted in the return va= lue > * > * Returns valid seccomp BPF response codes. > */ > -static u32 seccomp_run_filters(const struct seccomp_data *sd) > +static u32 seccomp_run_filters(const struct seccomp_data *sd, > + struct seccomp_filter **match) > { > struct seccomp_data sd_local; > u32 ret =3D SECCOMP_RET_ALLOW; My version of this patch initialized *match to f here. The reason I did that is because if BPF_PROG_RUN() returns RET_ALLOW for all filters, I didn't want *match to remain NULL when seccomp_run_filters() returns. FILTER_FLAG_LOG nor FILTER_FLAG_KILL_PROCESS would be affected by this because they don't care about RET_ALLOW actions but there could conceivably be a filter flag in the future that cares about RET_ALLOW and not initializing *match to the first filter could result in a latent bug for that filter flag. I'm fine with not adding the initialization since this is a hot path and it doesn't help any of the currently existing/planned filter flags but I wanted to at least mention it. Reviewed-by: Tyler Hicks Tyler > @@ -198,8 +200,10 @@ static u32 seccomp_run_filters(const struct seccom= p_data *sd) > for (; f; f =3D f->prev) { > u32 cur_ret =3D BPF_PROG_RUN(f->prog, sd); > =20 > - if ((cur_ret & SECCOMP_RET_ACTION) < (ret & SECCOMP_RET_ACTION)) > + if ((cur_ret & SECCOMP_RET_ACTION) < (ret & SECCOMP_RET_ACTION)) { > ret =3D cur_ret; > + *match =3D f; > + } > } > return ret; > } > @@ -566,6 +570,7 @@ static int __seccomp_filter(int this_syscall, const= struct seccomp_data *sd, > const bool recheck_after_trace) > { > u32 filter_ret, action; > + struct seccomp_filter *match =3D NULL; > int data; > =20 > /* > @@ -574,7 +579,7 @@ static int __seccomp_filter(int this_syscall, const= struct seccomp_data *sd, > */ > rmb(); > =20 > - filter_ret =3D seccomp_run_filters(sd); > + filter_ret =3D seccomp_run_filters(sd, &match); > data =3D filter_ret & SECCOMP_RET_DATA; > action =3D filter_ret & SECCOMP_RET_ACTION; > =20 >=20 --op9NH7wawAvILpcjR1kSRQmI5w77C4IlQ-- --agWPSeJBgenvDUmdhS9mG3B4VweSiOhFa Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBCgAGBQJZiQ35AAoJENaSAD2qAscK98MP/RD5XUmInMHQKLA3WbGErMwu Rj2NrHK7YXdLl16bCCvF9uz25Os87cIc2yXuBwq4ETlEjgfUZsbgvXHnXtiP4TJM h2NP0/d5sFTBbZ1cCM0rf3k7geUPX/ssix+FZp1ACheQ09m0Hli45AYyRTVuF5yQ 0ZtRxk5y6y14RHiTueHGG/EgZFJ06hrTzss8D94YdCWxbis3ZtfqI3We+t7LZgiq bTnCArfkszhxKwRqyjM6I9PTeSUfSj+TuipH9uDDegMgIj6gc2Nmuktcxpe7fe06 8JEyBB0r7Kx1SSpGITCWVaeIFcI0g005XbFkLc19l0TGrLGXVMgf6ubaM5PsIF+Z ny3th8H9uqEI9udv3PUiRJI8eOtcf5YaL3ejjIOsTe4os0DW5p0Mc08InuxtZRc3 kZCzVxou+rqvnpi1FStFpAxv0AhZxSQcR9PZp8Sw92InaOOmxzI+KgbakyPPgmMp 1bc7O3CBqBGwSBTOvm3jyojEwkhgFmrc25woYab9dt3lPLody49y4fI7bc4Ufjio gngEa7554N+JSoHHXOOcM4+cxOueu0rG1L2uion2kHY2UPu7ZNj8XZ6GRrb8vp4d NlOtJrWHloOGG2mp5IZEFeMq6Hgvj1mZatCPy0yMKzWmtU2Xp41mVK9f2Qo7RIiP eb5YCINlvE163qEQgoSS =5+cp -----END PGP SIGNATURE----- --agWPSeJBgenvDUmdhS9mG3B4VweSiOhFa--