Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sun, 23 May 2021 09:31:24 +0200
From:   John Wood <john.wood@gmx.com>
To:     Andi Kleen <ak@linux.intel.com>
Cc:     John Wood <john.wood@gmx.com>, Kees Cook <keescook@chromium.org>,
        Jann Horn <jannh@google.com>, Jonathan Corbet <corbet@lwn.net>,
        James Morris <jmorris@namei.org>,
        "Serge E. Hallyn" <serge@hallyn.com>,
        Shuah Khan <shuah@kernel.org>, valdis.kletnieks@vt.edu,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Randy Dunlap <rdunlap@infradead.org>,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-security-module@vger.kernel.org,
        linux-kselftest@vger.kernel.org, linux-hardening@vger.kernel.org,
        kernel-hardening@lists.openwall.com
Subject: Re: [PATCH v7 0/7] Fork brute force attack mitigation
Message-ID: <20210523073124.GA3762@ubuntu>
References: <20210521172414.69456-1-john.wood@gmx.com>
 <19903478-52e0-3829-0515-3e17669108f7@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <19903478-52e0-3829-0515-3e17669108f7@linux.intel.com>
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi,

On Fri, May 21, 2021 at 11:02:14AM -0700, Andi Kleen wrote:
>
> > Moreover, I think this solves another problem pointed out by Andi Klee=
n
> > during the v5 review [2] related to the possibility that a supervisor
> > respawns processes killed by the Brute LSM. He suggested adding some w=
ay so
> > a supervisor can know that a process has been killed by Brute and then
> > decide to respawn or not. So, now, the supervisor can read the brute x=
attr
> > of one executable and know if it is blocked by Brute and why (using th=
e
> > statistical data).
>
> It looks better now, Thank.
>
> One potential problem is that the supervisor might see the executable
> directly, but run it through some wrapper. In fact I suspect that will b=
e
> fairly common with complex daemons. So it couldn't directly look at the
> xattr. Might be useful to also pass this information through the wait*
> chain, so that the supervisor can directly collect it. That would need s=
ome
> extension to these system calls.
>
Could something like this help? (not tested)

diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_comp=
at.c
index 0e5d0a7e203b..409c9c4c40c0 100644
=2D-- a/arch/x86/kernel/signal_compat.c
+++ b/arch/x86/kernel/signal_compat.c
@@ -30,7 +30,7 @@ static inline void signal_compat_build_tests(void)
 	BUILD_BUG_ON(NSIGSEGV !=3D 9);
 	BUILD_BUG_ON(NSIGBUS  !=3D 5);
 	BUILD_BUG_ON(NSIGTRAP !=3D 6);
-	BUILD_BUG_ON(NSIGCHLD !=3D 6);
+	BUILD_BUG_ON(NSIGCHLD !=3D 7);
 	BUILD_BUG_ON(NSIGSYS  !=3D 2);

 	/* This is part of the ABI and can never change in size: */
diff --git a/include/brute/brute.h b/include/brute/brute.h
new file mode 100644
index 000000000000..1569bd495f94
=2D-- /dev/null
+++ b/include/brute/brute.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _BRUTE_H_
+#define _BRUTE_H_
+
+#include <linux/sched.h>
+
+#ifdef CONFIG_SECURITY_FORK_BRUTE
+bool brute_task_killed(struct task_struct *task);
+#else
+static inline bool brute_task_killed(struct task_struct *task) { return f=
alse; }
+#endif
+
+#endif /* _BRUTE_H_ */
diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic=
/siginfo.h
index 03d6f6d2c1fe..488abfdc7b0d 100644
=2D-- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -273,7 +273,8 @@ typedef struct siginfo {
 #define CLD_TRAPPED	4	/* traced child has trapped */
 #define CLD_STOPPED	5	/* child has stopped */
 #define CLD_CONTINUED	6	/* stopped child has continued */
-#define NSIGCHLD	6
+#define CLD_BRUTE	7	/* child was killed by brute LSM */
+#define NSIGCHLD	7

 /*
  * SIGPOLL (or any other signal without signal specific si_codes) si_code=
s
diff --git a/kernel/exit.c b/kernel/exit.c
index fd1c04193e18..69bcbd00d277 100644
=2D-- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -69,6 +69,8 @@
 #include <asm/unistd.h>
 #include <asm/mmu_context.h>

+#include <brute/brute.h>
+
 static void __unhash_process(struct task_struct *p, bool group_dead)
 {
 	nr_threads--;
@@ -1001,6 +1003,7 @@ static int wait_task_zombie(struct wait_opts *wo, st=
ruct task_struct *p)
 	pid_t pid =3D task_pid_vnr(p);
 	uid_t uid =3D from_kuid_munged(current_user_ns(), task_uid(p));
 	struct waitid_info *infop;
+	bool killed_by_brute =3D brute_task_killed(p);

 	if (!likely(wo->wo_flags & WEXITED))
 		return 0;
@@ -1114,7 +1117,8 @@ static int wait_task_zombie(struct wait_opts *wo, st=
ruct task_struct *p)
 			infop->cause =3D CLD_EXITED;
 			infop->status =3D status >> 8;
 		} else {
-			infop->cause =3D (status & 0x80) ? CLD_DUMPED : CLD_KILLED;
+			infop->cause =3D (status & 0x80) ? CLD_DUMPED :
+				killed_by_brute ? CLD_BRUTE : CLD_KILLED;
 			infop->status =3D status & 0x7f;
 		}
 		infop->pid =3D pid;
diff --git a/kernel/signal.c b/kernel/signal.c
index 62625ad98b14..f6c062b19563 100644
=2D-- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -55,6 +55,8 @@
 #include <asm/siginfo.h>
 #include <asm/cacheflush.h>

+#include <brute/brute.h>
+
 /*
  * SLAB caches for signal bits.
  */
@@ -1996,7 +1998,7 @@ bool do_notify_parent(struct task_struct *tsk, int s=
ig)
 	if (tsk->exit_code & 0x80)
 		info.si_code =3D CLD_DUMPED;
 	else if (tsk->exit_code & 0x7f)
-		info.si_code =3D CLD_KILLED;
+		info.si_code =3D brute_task_killed(tsk) ? CLD_BRUTE : CLD_KILLED;
 	else {
 		info.si_code =3D CLD_EXITED;
 		info.si_status =3D tsk->exit_code >> 8;

Thanks,
John Wood