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.5 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no 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 A9D77C433E2 for ; Sun, 13 Sep 2020 14:34:32 +0000 (UTC) Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by mail.kernel.org (Postfix) with SMTP id 44A5A2158C for ; Sun, 13 Sep 2020 14:34:30 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=gmx.net header.i=@gmx.net header.b="bDSoCpaW" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 44A5A2158C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=gmx.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kernel-hardening-return-19892-kernel-hardening=archiver.kernel.org@lists.openwall.com Received: (qmail 30565 invoked by uid 550); 13 Sep 2020 14:34:22 -0000 Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Received: (qmail 30545 invoked from network); 13 Sep 2020 14:34:22 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1600007610; bh=dHflCuyQDEYDcMpnPKECIFyNSbsiT8CytXlxu2bp71o=; h=X-UI-Sender-Class:Date:From:To:Cc:Subject:References:In-Reply-To; b=bDSoCpaW9dBsL2Re69QW0Ivh+uMs3Lhd+7aN2ghPtpPeOxQGgPuTPD9VBMY7PScxZ fqWvgD7YnoSPLFPa4dtja62tZuLzBqO6doYWbwW/XlIt2WSkJE4jg7CFoTE71eBj6X F3XnJTHec0KYINrIJadRACkr3dJPNUGbySNy2NZI= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Date: Sun, 13 Sep 2020 16:33:09 +0200 From: John Wood To: Kees Cook Cc: Jann Horn , kernel-hardening@lists.openwall.com, John Wood , Matthew Wilcox , Jonathan Corbet , Alexander Viro , Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Luis Chamberlain , Iurii Zaikin , James Morris , "Serge E. Hallyn" , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-security-module@vger.kernel.org Subject: Re: [RFC PATCH 4/6] security/fbfam: Add a new sysctl to control the crashing rate threshold Message-ID: <20200913143309.GA2873@ubuntu> References: <20200910202107.3799376-1-keescook@chromium.org> <20200910202107.3799376-5-keescook@chromium.org> <202009101612.18BAD0241D@keescook> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <202009101612.18BAD0241D@keescook> X-Provags-ID: V03:K1:g34bOsvNGvcdIL3vhD+B4Loro4lpz3d33Rzd1nJZQCi64UltDtY KxXkFlAvpWBqoGc9UEunLZ+B/OmAtjL1rp4M3xrRUyHVKn12OGhCe2skngRwsODg8cbLsHO OHdWHHQb5TJ5/xDAvrL28XWApVU8+vl92dQGrc+yL91F4QwOnGwgm5j3yViHD8UrqUh3mNd V/g/bnSsOi/9Xa3OzjkhQ== X-UI-Out-Filterresults: notjunk:1;V03:K0:0eHgbzh5VJg=:ZD7BwxwgOEUm0hOd/i4YrI Fy51LzGO2bzJr1jUdSDKUlZRJnVkZSwj1v8pheewD47iAw4hWoVTDV84BSunzJQF/BzMeDGBQ BQQycBZFdkD/yVLCuM4C9qE/aWPHMnO1cXT+uLP/rFM8gj4cJY6YlKWG+0dSEyGHUD+UiIe1n c9jjxKj+n2ZoKGLKOm+MTwA8eUY2AIDNXoG4xl72KUjSFxT9H3QSWMxuSfZsM7yhpFPw0mkSZ G9T+e13NT0Vs0ifMAg2UU+5062nwXlbFPKSjKYy+VscFYG9Xi+xT7KbN7BkUDSUxFM1O3G4/C ar+h0qHAFvl/0wMUsbsUsoPiz1zqdySLoFdDA8dJ1rRbGtQa901oUMH2QJUZFZBzY/xnAvax0 XudxWOSXMX0fhJ7g1HTMfAj406uzGLblplZMDti7X1im/8nsH6wtIlnAzkKUEtbwk/r/k9WfI MrTkTLeBtC9++q37GzWAcE0zxzx9c9x5KCof28AabcNOhMzXOtQgboC2u+2XP31G2MkYzK9L7 XCCs3f+n59WX0f1RZ030F7u6KJvoKRSH762jAqngmtuPXF3c8lMVGWNlWbd60lkK+gTvIGpRp SI0aS6YB9d6D+PqylBmjQLrlIK8l/pQK84QE2gMV++kDVmYKixwafFKSECZpsrTtr4SbVinkG E1F1h5shl6WEAei7/Kc4SQVRIYJSOs7pc2zqqyIgZxqNEWMkao/83buIqnkRsNQXBhC0MQx1M k+npfS97YzRpmC7xSOh2hB3slromS9JXDETWrjtbrADuJA/7ea/kiXUOyhum+Hzz94WBFJglR LqlaMDDqz5fiBCmDrK12azcDC7TBXAN+I9NrIWTkpsW49tEnLDHyuRlIbgFQxA34uMDYjwWhD OlRJXDyxv+0cMmwsxL+8DzkoOrcWonYaKTsbw9MlHSExjHzpHiya0bhXysKqq1sY6c7xG/ST7 zOMcy2Q+dCvqwxWAKv6P15p9OLD5I6rbTYW30fbyYWDYvxdbtQOGBXFg2/3SEED89zKsXNCWs 0n5KAUx9hnHyLRzNy0ify1BAt4vyTwu4bGefwpcYcWhaBmBl6jTw08Kzvtlqoe+V+hOnBN34f OpMI06rO35Rp3i4cbm1CdOI1H4vT8UjQ72N3OtLt5Jn/U/L46G7FRuTHr6XAdhqLE3lSPwlL7 q+gMYrHOdM419N0m7Hod+1AZixmf9zmx3sOv8f//MMVKRFtG11SaNrita8N5A1O/Tv+czH0wC c+PSGq3fJJhrf6H/p4UMFzzS6pt3K58QJnF0iaA== Content-Transfer-Encoding: quoted-printable Hi, more inline. On Thu, Sep 10, 2020 at 04:14:38PM -0700, Kees Cook wrote: > > diff --git a/include/fbfam/fbfam.h b/include/fbfam/fbfam.h > > index b5b7d1127a52..2cfe51d2b0d5 100644 > > --- a/include/fbfam/fbfam.h > > +++ b/include/fbfam/fbfam.h > > @@ -3,8 +3,12 @@ > > #define _FBFAM_H_ > > > > #include > > +#include > > > > #ifdef CONFIG_FBFAM > > +#ifdef CONFIG_SYSCTL > > +extern struct ctl_table fbfam_sysctls[]; > > +#endif > > Instead of doing the extern and adding to sysctl.c, this can all be done > directly (dynamically) from the fbfam.c file instead. Like Yama do in the yama_init_sysctl() function? As a reference code. > > int fbfam_fork(struct task_struct *child); > > int fbfam_execve(void); > > int fbfam_exit(void); > > diff --git a/kernel/sysctl.c b/kernel/sysctl.c > > index 09e70ee2332e..c3b4d737bef3 100644 > > --- a/kernel/sysctl.c > > +++ b/kernel/sysctl.c > > @@ -77,6 +77,8 @@ > > #include > > #include > > > > +#include > > + > > #ifdef CONFIG_X86 > > #include > > #include > > @@ -2660,6 +2662,13 @@ static struct ctl_table kern_table[] =3D { > > .extra1 =3D SYSCTL_ZERO, > > .extra2 =3D SYSCTL_ONE, > > }, > > +#endif > > +#ifdef CONFIG_FBFAM > > + { > > + .procname =3D "fbfam", > > + .mode =3D 0555, > > + .child =3D fbfam_sysctls, > > + }, > > #endif > > { } > > }; > > diff --git a/security/fbfam/Makefile b/security/fbfam/Makefile > > index f4b9f0b19c44..b8d5751ecea4 100644 > > --- a/security/fbfam/Makefile > > +++ b/security/fbfam/Makefile > > @@ -1,2 +1,3 @@ > > # SPDX-License-Identifier: GPL-2.0 > > obj-$(CONFIG_FBFAM) +=3D fbfam.o > > +obj-$(CONFIG_SYSCTL) +=3D sysctl.o > > diff --git a/security/fbfam/fbfam.c b/security/fbfam/fbfam.c > > index 0387f95f6408..9be4639b72eb 100644 > > --- a/security/fbfam/fbfam.c > > +++ b/security/fbfam/fbfam.c > > @@ -7,6 +7,17 @@ > > #include > > #include > > > > +/** > > + * sysctl_crashing_rate_threshold - Crashing rate threshold. > > + * > > + * The rate's units are in milliseconds per fault. > > + * > > + * A fork brute force attack will be detected if the application's cr= ashing rate > > + * falls under this threshold. So, the higher this value, the faster = an attack > > + * will be detected. > > + */ > > +unsigned long sysctl_crashing_rate_threshold =3D 30000; > > I would move the sysctls here, instead. (Also, the above should be > const.) If the above variable is const how the sysctl interface can modify it? I think it would be better to declare it as __read_mostly instead. What do you think? unsigned long sysctl_crashing_rate_threshold __read_mostly =3D 30000; > > + > > /** > > * struct fbfam_stats - Fork brute force attack mitigation statistics= . > > * @refc: Reference counter. > > diff --git a/security/fbfam/sysctl.c b/security/fbfam/sysctl.c > > new file mode 100644 > > index 000000000000..430323ad8e9f > > --- /dev/null > > +++ b/security/fbfam/sysctl.c > > @@ -0,0 +1,20 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +#include > > + > > +extern unsigned long sysctl_crashing_rate_threshold; > > +static unsigned long ulong_one =3D 1; > > +static unsigned long ulong_max =3D ULONG_MAX; > > + > > +struct ctl_table fbfam_sysctls[] =3D { > > + { > > + .procname =3D "crashing_rate_threshold", > > + .data =3D &sysctl_crashing_rate_threshold, > > + .maxlen =3D sizeof(sysctl_crashing_rate_threshold), > > + .mode =3D 0644, > > + .proc_handler =3D proc_doulongvec_minmax, > > + .extra1 =3D &ulong_one, > > + .extra2 =3D &ulong_max, > > + }, > > + { } > > +}; > > I wouldn't bother splitting this into a separate file. (Just leave it in > fbfam.c) > > -- > Kees Cook Thanks, John Wood