Hi,
So this is the fix for the bug that actually prevented me to integrate
HID-BPF in v6.2.
While testing the code base with LLVM, I realized that clang was smarter
than I expected it to be, and it sometimes inlined a function or not
depending on the branch. This lead to segfaults because my current code
in linux-next is messing up the bpf programs refcounts assuming that I
had enough observability over the kernel.
So I came back to the drawing board and realized that what I was missing
was exactly a bpf_link, to represent the attachment of a bpf program to
a HID device. This is the bulk of the series, in patch 6/9.
The other patches are cleanups, tests, and also the addition of the
vmtests.sh script I run locally, largely inspired by the one in the bpf
selftests dir. This allows very fast development of HID-BPF, assuming we
have tests that cover the bugs :)
changes in v2:
- took Alexei's remarks into account and renamed the indexes into
prog_table_index and hid_table_index
- fixed unused function as reported by the Intel kbuild bot
Cheers,
Benjamin
Benjamin Tissoires (9):
selftests: hid: add vmtest.sh
selftests: hid: allow to compile hid_bpf with LLVM
selftests: hid: attach/detach 2 bpf programs, not just one
selftests: hid: ensure the program is correctly pinned
selftests: hid: prepare tests for HID_BPF API change
HID: bpf: rework how programs are attached and stored in the kernel
selftests: hid: enforce new attach API
HID: bpf: clean up entrypoint
HID: bpf: reorder BPF registration
Documentation/hid/hid-bpf.rst | 12 +-
drivers/hid/bpf/entrypoints/entrypoints.bpf.c | 9 -
.../hid/bpf/entrypoints/entrypoints.lskel.h | 188 ++++--------
drivers/hid/bpf/hid_bpf_dispatch.c | 28 +-
drivers/hid/bpf/hid_bpf_dispatch.h | 3 -
drivers/hid/bpf/hid_bpf_jmp_table.c | 129 ++++----
include/linux/hid_bpf.h | 7 +
tools/testing/selftests/hid/.gitignore | 1 +
tools/testing/selftests/hid/Makefile | 10 +-
tools/testing/selftests/hid/config.common | 241 +++++++++++++++
tools/testing/selftests/hid/config.x86_64 | 4 +
tools/testing/selftests/hid/hid_bpf.c | 32 +-
tools/testing/selftests/hid/progs/hid.c | 13 +
tools/testing/selftests/hid/vmtest.sh | 284 ++++++++++++++++++
14 files changed, 728 insertions(+), 233 deletions(-)
create mode 100644 tools/testing/selftests/hid/config.common
create mode 100644 tools/testing/selftests/hid/config.x86_64
create mode 100755 tools/testing/selftests/hid/vmtest.sh
--
2.38.1
On Wed, Jan 18, 2023 at 02:41:00PM -0500, Gregory Price wrote:
> ---------- Forwarded message ---------
> From: Peter Zijlstra <peterz(a)infradead.org>
> Date: Wed, Jan 18, 2023 at 12:16 PM
> Subject: Re: [PATCH 1/3] ptrace,syscall_user_dispatch: Implement Syscall
> User Dispatch Suspension
> To: Gregory Price <gourry.memverge(a)gmail.com>
>
>
> On Mon, Jan 09, 2023 at 10:33:46AM -0500, Gregory Price wrote:
> > @@ -36,6 +37,10 @@ bool syscall_user_dispatch(struct pt_regs *regs)
> > struct syscall_user_dispatch *sd = ¤t->syscall_dispatch;
> > char state;
> >
> > + if (IS_ENABLED(CONFIG_CHECKPOINT_RESTORE) &&
> > + unlikely(current->ptrace &
> PT_SUSPEND_SYSCALL_USER_DISPATCH))
> > + return false;
> > +
> > if (likely(instruction_pointer(regs) - sd->offset < sd->len))
> > return false;
> >
>
> So by making syscall_user_dispatch() return false, we'll make
> syscall_trace_enter() continue to handle things, and supposedly you want
> to land in ptrace_report_syscall_entry(), right?
>
> ... snip ...
>
> Should setting this then not also depend on having
> SYSCALL_WORK_SYSCALL_TRACE set? Because without that, you get 'funny'
> things.
Hm, this is an interesting question. My thoughts are that I want the
process to handle the syscall as-if syscall user dispatch was not
present at all, regardless of SYSCALL_TRACE.
This is because some software, like CRIU, actually injects syscalls to
run in the context of the software in an effort to collect resources.
So I actually *want* those 'funny' things to occur, because they're most
likely intentional. I don't necessarily want to intercept system calls
that subsequently occur (although i might).
So if this feature required SYSCALL_TRACE, you would no longer be able
to inject system calls ala CRIU.
That's also my understanding of the SECCOMP_SUSPEND feature as well,
it's intended specifically to allow *otherwise disallowed* syscalls to
be injected into the process and SECCOMP bypassed. (in this case,
SECCOMP_SUSPEND requires root for exactly this reason).
Syscall user dispatch makes it possible to cleanly intercept system
calls from user-land. However, most transparent checkpoint software
presently leverages some combination of ptrace and system call
injection to place software in a ready-to-checkpoint state.
If Syscall User Dispatch is enabled at the time of being quiesced,
injected system calls will subsequently be interposed upon and
dispatched to the task's signal handler.
This patch set implements 3 features to enable software such as CRIU
to cleanly interpose upon software leveraging syscall user dispatch.
- Implement PTRACE_O_SUSPEND_SYSCALL_USER_DISPATCH, akin to a similar
feature for SECCOMP. This allows a ptracer to temporarily disable
syscall user dispatch, making syscall injection possible.
- Implement an fs/proc extension that reports whether Syscall User
Dispatch is being used in proc/status. A similar value is present
for SECCOMP, and is used to determine whether special logic is
needed during checkpoint/resume.
- Implement a getter interface for Syscall User Dispatch config info.
To resume successfully, the checkpoint/resume software has to
save and restore this information. Presently this configuration
is write-only, with no way for C/R software to save it.
Signed-off-by: Gregory Price <gregory.price(a)memverge.com>
Gregory Price (3):
ptrace,syscall_user_dispatch: Implement Syscall User Dispatch
Suspension
fs/proc/array: Add Syscall User Dispatch to proc status
prctl,syscall_user_dispatch: add a getter for configuration info
.../admin-guide/syscall-user-dispatch.rst | 18 +++++++
fs/proc/array.c | 8 +++
include/linux/ptrace.h | 2 +
include/linux/syscall_user_dispatch.h | 7 +++
include/uapi/linux/prctl.h | 3 ++
include/uapi/linux/ptrace.h | 6 ++-
kernel/entry/syscall_user_dispatch.c | 19 +++++++
kernel/ptrace.c | 5 ++
kernel/sys.c | 4 ++
.../syscall_user_dispatch/sud_test.c | 54 +++++++++++++++++++
10 files changed, 125 insertions(+), 1 deletion(-)
--
2.37.3
The KVM rseq test is failing to build in -next due to a commit merged
from the tip tree which adds a wrapper for sys_getcpu() to the rseq
kselftests, conflicting with the wrapper already included in the KVM
selftest:
rseq_test.c:48:13: error: conflicting types for 'sys_getcpu'
48 | static void sys_getcpu(unsigned *cpu)
| ^~~~~~~~~~
In file included from rseq_test.c:23:
../rseq/rseq.c:82:12: note: previous definition of 'sys_getcpu' was here
82 | static int sys_getcpu(unsigned *cpu, unsigned *node)
| ^~~~~~~~~~
Fix this by removing the local wrapper and moving the result check up to
the caller.
Fixes: 99babd04b250 ("selftests/rseq: Implement rseq numa node id field selftest")
Signed-off-by: Mark Brown <broonie(a)kernel.org>
---
This will need to go via the tip tree due to the breaking change being
there.
---
tools/testing/selftests/kvm/rseq_test.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/tools/testing/selftests/kvm/rseq_test.c b/tools/testing/selftests/kvm/rseq_test.c
index 3045fdf9bdf5..f74e76d03b7e 100644
--- a/tools/testing/selftests/kvm/rseq_test.c
+++ b/tools/testing/selftests/kvm/rseq_test.c
@@ -41,18 +41,6 @@ static void guest_code(void)
GUEST_SYNC(0);
}
-/*
- * We have to perform direct system call for getcpu() because it's
- * not available until glic 2.29.
- */
-static void sys_getcpu(unsigned *cpu)
-{
- int r;
-
- r = syscall(__NR_getcpu, cpu, NULL, NULL);
- TEST_ASSERT(!r, "getcpu failed, errno = %d (%s)", errno, strerror(errno));
-}
-
static int next_cpu(int cpu)
{
/*
@@ -249,7 +237,9 @@ int main(int argc, char *argv[])
* across the seq_cnt reads.
*/
smp_rmb();
- sys_getcpu(&cpu);
+ r = sys_getcpu(&cpu, NULL);
+ TEST_ASSERT(!r, "getcpu failed, errno = %d (%s)",
+ errno, strerror(errno));
rseq_cpu = rseq_current_cpu_raw();
smp_rmb();
} while (snapshot != atomic_read(&seq_cnt));
---
base-commit: 469a89fd3bb73bb2eea628da2b3e0f695f80b7ce
change-id: 20230106-fix-kvm-rseq-build-41ac58ba1d27
Best regards,
--
Mark Brown <broonie(a)kernel.org>
The common layout for kbuild messages is as follows:
- 2 spaces
- 7 or more characters for the action
- 1 space
- name of the file being built/generated
The custom message formatting included an additional space in the action
part, which leads to misalignments with the rest of kbuild.
To: Alexei Starovoitov <ast(a)kernel.org>
To: Daniel Borkmann <daniel(a)iogearbox.net>
To: Andrii Nakryiko <andrii(a)kernel.org>
To: Martin KaFai Lau <martin.lau(a)linux.dev>
To: Song Liu <song(a)kernel.org>
To: Yonghong Song <yhs(a)fb.com>
To: John Fastabend <john.fastabend(a)gmail.com>
To: KP Singh <kpsingh(a)kernel.org>
To: Stanislav Fomichev <sdf(a)google.com>
To: Hao Luo <haoluo(a)google.com>
To: Jiri Olsa <jolsa(a)kernel.org>
To: Mykola Lysenko <mykolal(a)fb.com>
To: Shuah Khan <shuah(a)kernel.org>
Cc: bpf(a)vger.kernel.org
Cc: linux-kselftest(a)vger.kernel.org
Cc: linux-kernel(a)vger.kernel.org
To: Masahiro Yamada <masahiroy(a)kernel.org>
Cc: linux-kbuild(a)vger.kernel.org
Signed-off-by: Thomas Weißschuh <linux(a)weissschuh.net>
---
Thomas Weißschuh (3):
selftests/bpf: align kbuild messages to standard
bpf: iterators: align kbuild messages to standard
tools/resolve_btfids: align kbuild messages to standard
kernel/bpf/preload/iterators/Makefile | 2 +-
tools/bpf/resolve_btfids/Makefile | 2 +-
tools/testing/selftests/bpf/Makefile | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
---
base-commit: c1649ec55708ae42091a2f1bca1ab49ecd722d55
change-id: 20230118-kbuild-alignment-ca1ce98ea566
Best regards,
--
Thomas Weißschuh <linux(a)weissschuh.net>
From: Ammar Faizi <ammarfaizi2(a)gnuweeb.org>
Hi Willy,
On top of the series titled "nolibc auxiliary vector retrieval support".
The prerequisite patches of this series are in that series.
This is v2 of nolibc signal handling support. It adds signal handling
support to the nolibc subsystem:
1) Initial implementation of nolibc sigaction(2) function.
`sigaction()` needs an architecture-dependent "signal trampoline"
function that invokes __rt_sigreturn syscall to resume the process
after a signal gets handled.
The "signal trampoline" function is called `__restore_rt` in this
implementation. The naming `__restore_rt` is important for GDB. It
also has to be given a special optimization attribute
"omit-frame-pointer" to prevent the compiler from creating a stack
frame that makes the `%rsp` value no longer points to the `struct
rt_sigframe` that the kernel constructed.
2) signal(2) function.
signal() function is the simpler version of sigaction(). Unlike
sigaction(), which fully controls the struct sigaction, the caller
only cares about the sa_handler when calling the signal() function.
signal() internally calls sigaction().
3) More selftests.
This series also adds selftests for:
- fork(2)
- sigaction(2)
- signal(2)
Side note for __restore_rt:
This has been tested on x86-64 arch and `__restore_rt` generates the
correct code. The `__restore_rt` codegen correctness on other
architectures need to be evaluated as well. If it can't generate the
correct code, it has to be written in inline Assembly.
The current codegen for __restore_rt looks like this (gcc (Ubuntu 11.3.0-1ubuntu1~22.04) 11.3.0):
00000000004038e3 <__restore_rt>:
4038e3: endbr64
4038e7: mov $0xf,%eax
4038ec: syscall
## Changes since v2:
- Fix unintentionally squashed patch. The signal() selftest patch
was squashed into the sigaction selftest patch.
## Changes since RFC v1:
- Separate getpagesize() series.
- Write __restore_rt function in C instead of in inline Assembly.
Signed-off-by: Ammar Faizi <ammarfaizi2(a)gnuweeb.org>
---
Ammar Faizi (5):
nolibc/sys: Implement `sigaction(2)` function
nolibc/sys: Implement `signal(2)` function
selftests/nolibc: Add `fork(2)` selftest
selftests/nolibc: Add `sigaction(2)` selftest
selftests/nolibc: Add `signal(2)` selftest
tools/include/nolibc/sys.h | 97 +++++++++++
tools/testing/selftests/nolibc/nolibc-test.c | 172 +++++++++++++++++++
2 files changed, 269 insertions(+)
base-commit: b6887ec8b0b0c78db414b78e329bf2ce234dedd5
prerequisite-patch-id: 8dd0ca8ecee1732d8f5c0b233f8231dda6ab0d22
prerequisite-patch-id: ff4c08615ebbdc1a04ce39f39f99387ee46b2b31
prerequisite-patch-id: af837a829263849331eb6d73701afd7903146055
--
Ammar Faizi
On 2023-01-16 14:40, kernel test robot wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git sched/core
> head: 79ba1e607d68178db7d3fe4f6a4aa38f06805e7b
> commit: 03f5c0272d1b59343144e199becc911dae52c37e [7/28] selftests/rseq: Use ELF auxiliary vector for extensible rseq
> compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
> reproduce:
> # https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=03f5…
> git remote add tip https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
> git fetch --no-tags tip sched/core
> git checkout 03f5c0272d1b59343144e199becc911dae52c37e
> make O=/tmp/kselftest headers
> make O=/tmp/kselftest -C tools/testing/selftests
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp(a)intel.com>
In order to fix this, I need to change -I../../../../usr/include/ for
$(KHDR_INCLUDES) in tools/testing/selftests/rseq/Makefile
I can find 25 odd uses of the same pattern in the kernel selftests.
Should I fix them all in one go ?
grep -r "../../../../usr/include/" tools/testing/selftests/ | wc -l
25
AFAIU it typically works just because the build system happens to have
recent enough kernel headers installed in the root environment.
Thanks,
Mathieu
>
> All errors (new ones prefixed by >>):
>
> rseq.c: In function 'get_rseq_feature_size':
>>> rseq.c:139:37: error: 'AT_RSEQ_ALIGN' undeclared (first use in this function); did you mean 'R_SH_ALIGN'?
> 139 | auxv_rseq_align = getauxval(AT_RSEQ_ALIGN);
> | ^~~~~~~~~~~~~
> | R_SH_ALIGN
> rseq.c:139:37: note: each undeclared identifier is reported only once for each function it appears in
>>> rseq.c:142:44: error: 'AT_RSEQ_FEATURE_SIZE' undeclared (first use in this function); did you mean 'ORIG_RSEQ_FEATURE_SIZE'?
> 142 | auxv_rseq_feature_size = getauxval(AT_RSEQ_FEATURE_SIZE);
> | ^~~~~~~~~~~~~~~~~~~~
> | ORIG_RSEQ_FEATURE_SIZE
>
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com