From: Alexei Starovoitov crypto_enqueue_request() returns three codes. Only -ENOSPC is a failure: int err = -EINPROGRESS; if (unlikely(queue->qlen >= queue->max_qlen)) { if (!(request->flags & CRYPTO_TFM_REQ_MAY_BACKLOG)) { err = -ENOSPC; goto out; } err = -EBUSY; .. } queue->qlen++; list_add_tail(&request->list, &queue->list); out: return err; -EBUSY says that the request is queued on the backlog. crypto_wait_req() waits for the completion after -EINPROGRESS and after -EBUSY. The tag of the function is 'bpf_fmodret_retval:-28,-16'. An fmod_ret program that returns -EBUSY skips the list_add_tail(). The caller then waits for a completion that never comes. means_failure() cannot drop -EBUSY for all functions. In most of them it is a plain failure. In an x86-64 GCC build 645 of the 11432 tagged functions list it, and it is the only error of 205 of them. ccp_enqueue_cmd() and qat_alg_send_message_maybacklog() have such a backlog too. All three functions also return -EINPROGRESS. The rule: -EBUSY is not in the list if the function returns -EINPROGRESS too. crypto_enqueue_request() stays a target with -ENOSPC. list -EBUSY lose it x86-64 GCC 645 of 11432 1 x86-64 clang 674 of 12096 2 arm64 clang 638 of 10954 2 No function loses its tag. The second function is seccomp_notify_ioctl(). Clang inlines seccomp_notify_addfd() there. Its -EBUSY is a plain failure next to an -EINPROGRESS of another kind. It keeps five other errors. Reported-by: David Windsor Closes: https://lore.kernel.org/bpf/CAEXv5_i17dJmGp3y_0U_f_xK9-SyC9M4GkLCx=pT=4VoXJ4abg@mail.gmail.com/ Fixes: 4dd950adacad ("resolve_btfids: Tag the functions that return errors") Signed-off-by: Alexei Starovoitov --- tools/bpf/resolve_btfids/fmodret.c | 13 +++++++++++-- .../selftests/bpf/prog_tests/fmodret_retval.c | 1 + .../selftests/bpf/test_kmods/bpf_testmod_retval.c | 8 ++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/tools/bpf/resolve_btfids/fmodret.c b/tools/bpf/resolve_btfids/fmodret.c index 0751519b95d2..e1afdb6dedd5 100644 --- a/tools/bpf/resolve_btfids/fmodret.c +++ b/tools/bpf/resolve_btfids/fmodret.c @@ -61,9 +61,15 @@ static bool fits_trampoline(const struct btf *btf, const struct btf_type *func) * -EAGAIN, -EMSGSIZE and -EINPROGRESS tell the caller to retry, to go on or to * wait, not that the function failed. Their numbers are those of the kernel, in * asm-generic/errno.h. The errno.h of the build host can have other numbers. + * + * ASYNC: the function returns -EINPROGRESS too. Then its -EBUSY is no failure + * either. In the crypto API it says that the request is queued on the backlog: + * crypto_wait_req() waits after both. */ -static bool means_failure(int error) +static bool means_failure(int error, bool async) { + if (async && error == -16) + return false; return error > -512 && error != -11 && error != -90 && error != -115; } @@ -82,13 +88,16 @@ static int cmp_error(const void *va, const void *vb) */ static bool format_tag(char *tag, const int64_t *values, size_t nr, __u32 size) { + bool async = false; size_t i, n = 0; int errors[nr]; + for (i = 0; i < nr; i++) + async |= (int)values[i] == -115; for (i = 0; i < nr; i++) { int error = values[i]; - if ((size == 4 || values[i] == error) && means_failure(error)) + if ((size == 4 || values[i] == error) && means_failure(error, async)) errors[n++] = error; } qsort(errors, n, sizeof(*errors), cmp_error); diff --git a/tools/testing/selftests/bpf/prog_tests/fmodret_retval.c b/tools/testing/selftests/bpf/prog_tests/fmodret_retval.c index 1db55d8d8d67..281c6192e570 100644 --- a/tools/testing/selftests/bpf/prog_tests/fmodret_retval.c +++ b/tools/testing/selftests/bpf/prog_tests/fmodret_retval.c @@ -17,6 +17,7 @@ static const struct { { "bpf_testmod_retval_minus_one", "" }, { "bpf_testmod_retval_eperm", "-2,-1" }, { "bpf_testmod_retval_denied", "-13" }, + { "bpf_testmod_retval_backlog", "-28" }, { "bpf_testmod_retval_effect", "-22" }, { "bpf_testmod_retval_effect_late", "-22" }, { "bpf_testmod_retval_effect_two_regs", "-22" }, diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_retval.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_retval.c index c6263d7e65be..85722e48d34c 100644 --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_retval.c +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_retval.c @@ -51,6 +51,14 @@ static __used noinline int bpf_testmod_retval_denied(int a, int b, int c, int d) return d ? -ERESTARTSYS : -EIOCBQUEUED; } +/* As crypto_enqueue_request(): next to -EINPROGRESS, -EBUSY is "queued on the backlog" */ +static __used noinline int bpf_testmod_retval_backlog(int free, int may_backlog) +{ + if (free) + return -EINPROGRESS; + return may_backlog ? -EBUSY : -ENOSPC; +} + /* A caller which gets -EBUSY relies on the unlock */ static __used noinline int bpf_testmod_retval_effect(struct mutex *lock, int value) { base-commit: 3a4161c87e356d33e179076a5194d6f407cda567 -- 2.55.0