[PATCH bpf-next] resolve_btfids: Don't list -EBUSY next to -EINPROGRESS

[PATCH bpf-next] resolve_btfids: Don't list -EBUSY next to -EINPROGRESS

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

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 <dwindsor@gmail.com>
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 <ast@kernel.org>
---
 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

Re: [PATCH bpf-next] resolve_btfids: Don't list -EBUSY next to -EINPROGRESS

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3560 bytes --]

> resolve_btfids: Don't list -EBUSY next to -EINPROGRESS

TL;DR: The -EBUSY/-EINPROGRESS fix may be incomplete: async is only detected
from local constants, so functions getting -EINPROGRESS from callees (e.g.
sec_bd_send) could keep -EBUSY in fmod_ret tags, possibly hanging backlog
callers.

> diff --git a/tools/bpf/resolve_btfids/fmodret.c b/tools/bpf/resolve_btfids/fmodret.c
> --- a/tools/bpf/resolve_btfids/fmodret.c
> +++ b/tools/bpf/resolve_btfids/fmodret.c

[ ... ]

> @@ -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;
>  	}

Is this fix complete?  The comment on means_failure() says "ASYNC: the
function returns -EINPROGRESS too", but async is only set when -EINPROGRESS
is in values[].  That array holds only the constants scan_func() found in
the function's own code that reach the return without an effect:

    /* fmodret_scan.c:scan_func() */
    if (insn->kind != TEXT_MOV_IMM && insn->kind != TEXT_SELECT_IMM)
            continue;
    if (flow->reach[i].path == TEXT_EFFECT_PATH)
            continue;

A function that queues a request on the backlog and returns a local
-EBUSY, but gets its -EINPROGRESS from an out-of-line callee or after a
void callee that takes an argument pointer, would keep -16 in its tag.

For example, sec_bd_send() in drivers/crypto/hisilicon/sec2 has this
once sec_alg_send_message_maybacklog() and sec_alg_try_enqueue() are
inlined:

    if (ret == -EBUSY)
            list_add_tail(&req->list, &qp->backlog.list);
    ...
    return ret;

Its only -EINPROGRESS is "return -EINPROGRESS;" inside qp_send_message(),
a static function with four call sites that GCC -O2 is unlikely to inline.
Then values[] is {-16}, async stays false, and the tag is
'bpf_fmodret_retval:-16'.  sec_process() treats -EBUSY from bd_send as
queued:

    ret = ctx->req_op->bd_send(ctx, req);
    if (unlikely((ret != -EBUSY && ret != -EINPROGRESS))) {

so an fmod_ret program returning -EBUSY there would skip the
list_add_tail(), and a MAY_BACKLOG caller such as crypto_wait_req() would
wait for a completion that never comes.

cc_send_request() in ccree looks similar.  Its -EINPROGRESS follows a
void callee that takes the drvdata argument:

    cc_do_send_request(drvdata, cc_req, desc, len, false);
    rc = -EINPROGRESS;

When cc_do_send_request() or enqueue_seq() stays out of line, that is an
effect and the -115 is dropped, while the backlog "return -EBUSY;" after
the inlined cc_enqueue_backlog() reaches the return on a clean path.

Whether a given function is affected depends on the compiler's inlining,
and the counts in the commit message cover only vmlinux, not modules.

Could async be decided from something other than the effect-filtered local
constants?  For example, by also looking at the constants of direct
callees, or by not filtering -EINPROGRESS by effect path when computing
async.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/43655020416

Re: [PATCH bpf-next] resolve_btfids: Don't list -EBUSY next to -EINPROGRESS

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Sat, 14 Nov 2026 23:27:54 +0000 you wrote:
> From: Alexei Starovoitov <ast@kernel.org>
> 
> 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;
> 
> [...]

Here is the summary with links:
  - [bpf-next] resolve_btfids: Don't list -EBUSY next to -EINPROGRESS
    https://git.kernel.org/bpf/bpf-next/c/8835113107b2

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html