[PATCH bpf-next] bpf: Use sha1() instead of sha1_transform() in bpf_prog_calc_tag()

[PATCH bpf-next] bpf: Use sha1() instead of sha1_transform() in bpf_prog_calc_tag()

[Eric Biggers]

Now that there's a proper SHA-1 library API, just use that instead of
the low-level SHA-1 compression function.  This eliminates the need for
bpf_prog_calc_tag() to implement the SHA-1 padding itself.  No
functional change; the computed tags remain the same.

Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 include/linux/filter.h |  6 -----
 kernel/bpf/core.c      | 50 ++++++++----------------------------------
 2 files changed, 9 insertions(+), 47 deletions(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index 1e7fd3ee759e0..1bcc81ab32273 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -995,16 +995,10 @@ void bpf_prog_change_xdp(struct bpf_prog *prev_prog, struct bpf_prog *prog);
 static inline u32 bpf_prog_insn_size(const struct bpf_prog *prog)
 {
 	return prog->len * sizeof(struct bpf_insn);
 }
 
-static inline u32 bpf_prog_tag_scratch_size(const struct bpf_prog *prog)
-{
-	return round_up(bpf_prog_insn_size(prog) +
-			sizeof(__be64) + 1, SHA1_BLOCK_SIZE);
-}
-
 static inline unsigned int bpf_prog_size(unsigned int proglen)
 {
 	return max(sizeof(struct bpf_prog),
 		   offsetof(struct bpf_prog, insns[proglen]));
 }
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 5d1650af899d0..ef01cc644a965 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -16,10 +16,11 @@
  * Andi Kleen - Fix a few bad bugs and races.
  * Kris Katterjohn - Added many additional checks in bpf_check_classic()
  */
 
 #include <uapi/linux/btf.h>
+#include <crypto/sha1.h>
 #include <linux/filter.h>
 #include <linux/skbuff.h>
 #include <linux/vmalloc.h>
 #include <linux/prandom.h>
 #include <linux/bpf.h>
@@ -291,32 +292,23 @@ void __bpf_prog_free(struct bpf_prog *fp)
 	vfree(fp);
 }
 
 int bpf_prog_calc_tag(struct bpf_prog *fp)
 {
-	const u32 bits_offset = SHA1_BLOCK_SIZE - sizeof(__be64);
-	u32 raw_size = bpf_prog_tag_scratch_size(fp);
-	u32 digest[SHA1_DIGEST_WORDS];
-	u32 ws[SHA1_WORKSPACE_WORDS];
-	u32 i, bsize, psize, blocks;
+	size_t size = bpf_prog_insn_size(fp);
+	u8 digest[SHA1_DIGEST_SIZE];
 	struct bpf_insn *dst;
 	bool was_ld_map;
-	u8 *raw, *todo;
-	__be32 *result;
-	__be64 *bits;
+	u32 i;
 
-	raw = vmalloc(raw_size);
-	if (!raw)
+	dst = vmalloc(size);
+	if (!dst)
 		return -ENOMEM;
 
-	sha1_init_raw(digest);
-	memset(ws, 0, sizeof(ws));
-
 	/* We need to take out the map fd for the digest calculation
 	 * since they are unstable from user space side.
 	 */
-	dst = (void *)raw;
 	for (i = 0, was_ld_map = false; i < fp->len; i++) {
 		dst[i] = fp->insnsi[i];
 		if (!was_ld_map &&
 		    dst[i].code == (BPF_LD | BPF_IMM | BPF_DW) &&
 		    (dst[i].src_reg == BPF_PSEUDO_MAP_FD ||
@@ -332,37 +324,13 @@ int bpf_prog_calc_tag(struct bpf_prog *fp)
 			dst[i].imm = 0;
 		} else {
 			was_ld_map = false;
 		}
 	}
-
-	psize = bpf_prog_insn_size(fp);
-	memset(&raw[psize], 0, raw_size - psize);
-	raw[psize++] = 0x80;
-
-	bsize  = round_up(psize, SHA1_BLOCK_SIZE);
-	blocks = bsize / SHA1_BLOCK_SIZE;
-	todo   = raw;
-	if (bsize - psize >= sizeof(__be64)) {
-		bits = (__be64 *)(todo + bsize - sizeof(__be64));
-	} else {
-		bits = (__be64 *)(todo + bsize + bits_offset);
-		blocks++;
-	}
-	*bits = cpu_to_be64((psize - 1) << 3);
-
-	while (blocks--) {
-		sha1_transform(digest, todo, ws);
-		todo += SHA1_BLOCK_SIZE;
-	}
-
-	result = (__force __be32 *)digest;
-	for (i = 0; i < SHA1_DIGEST_WORDS; i++)
-		result[i] = cpu_to_be32(digest[i]);
-	memcpy(fp->tag, result, sizeof(fp->tag));
-
-	vfree(raw);
+	sha1((const u8 *)dst, size, digest);
+	memcpy(fp->tag, digest, sizeof(fp->tag));
+	vfree(dst);
 	return 0;
 }
 
 static int bpf_adj_delta_to_imm(struct bpf_insn *insn, u32 pos, s32 end_old,
 				s32 end_new, s32 curr, const bool probe_pass)

base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
-- 
2.50.1

Re: [PATCH bpf-next] bpf: Use sha1() instead of sha1_transform() in bpf_prog_calc_tag()

[Eduard Zingerman]

On Mon, 2025-08-11 at 13:16 -0700, Eric Biggers wrote:
> Now that there's a proper SHA-1 library API, just use that instead of
> the low-level SHA-1 compression function.  This eliminates the need for
> bpf_prog_calc_tag() to implement the SHA-1 padding itself.  No
> functional change; the computed tags remain the same.
> 
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---

The logic seem to unchanged.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

[...]

Re: [PATCH bpf-next] bpf: Use sha1() instead of sha1_transform() in bpf_prog_calc_tag()

[Song Liu]

On Mon, Aug 11, 2025 at 1:17 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> Now that there's a proper SHA-1 library API, just use that instead of
> the low-level SHA-1 compression function.  This eliminates the need for
> bpf_prog_calc_tag() to implement the SHA-1 padding itself.  No
> functional change; the computed tags remain the same.
>
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---
>  include/linux/filter.h |  6 -----
>  kernel/bpf/core.c      | 50 ++++++++----------------------------------
>  2 files changed, 9 insertions(+), 47 deletions(-)

Nice clean up!

It appears this patch changes the sha1 of some programs, but not
some other programs. For example, sha1 of program
test_task_kfunc_flavor_relo_not_found from task_kfunc_success.bpf.o
stays the same before and after the patch, while other programs from
task_kfunc_success.bpf.o have different sha1 after the patch.

Is this expected?

Thanks,
Song

Re: [PATCH bpf-next] bpf: Use sha1() instead of sha1_transform() in bpf_prog_calc_tag()

[Eric Biggers]

On Mon, Aug 11, 2025 at 05:57:58PM -0700, Song Liu wrote:
> On Mon, Aug 11, 2025 at 1:17 PM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > Now that there's a proper SHA-1 library API, just use that instead of
> > the low-level SHA-1 compression function.  This eliminates the need for
> > bpf_prog_calc_tag() to implement the SHA-1 padding itself.  No
> > functional change; the computed tags remain the same.
> >
> > Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> > ---
> >  include/linux/filter.h |  6 -----
> >  kernel/bpf/core.c      | 50 ++++++++----------------------------------
> >  2 files changed, 9 insertions(+), 47 deletions(-)
> 
> Nice clean up!
> 
> It appears this patch changes the sha1 of some programs, but not
> some other programs. For example, sha1 of program
> test_task_kfunc_flavor_relo_not_found from task_kfunc_success.bpf.o
> stays the same before and after the patch, while other programs from
> task_kfunc_success.bpf.o have different sha1 after the patch.
> 
> Is this expected?

I don't see how the behavior could have changed.  The previous code
calculated the SHA-1 value correctly, just in a hard-to-read way.  Is it
possible that those BPF programs changed between your two tests?  Did
you recompile them?

- Eric

Re: [PATCH bpf-next] bpf: Use sha1() instead of sha1_transform() in bpf_prog_calc_tag()

[Andrii Nakryiko]

On Mon, Aug 11, 2025 at 5:58 PM Song Liu <song@kernel.org> wrote:
>
> On Mon, Aug 11, 2025 at 1:17 PM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > Now that there's a proper SHA-1 library API, just use that instead of
> > the low-level SHA-1 compression function.  This eliminates the need for
> > bpf_prog_calc_tag() to implement the SHA-1 padding itself.  No
> > functional change; the computed tags remain the same.
> >
> > Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> > ---
> >  include/linux/filter.h |  6 -----
> >  kernel/bpf/core.c      | 50 ++++++++----------------------------------
> >  2 files changed, 9 insertions(+), 47 deletions(-)
>
> Nice clean up!
>
> It appears this patch changes the sha1 of some programs, but not
> some other programs. For example, sha1 of program
> test_task_kfunc_flavor_relo_not_found from task_kfunc_success.bpf.o
> stays the same before and after the patch, while other programs from
> task_kfunc_success.bpf.o have different sha1 after the patch.
>
> Is this expected?
>

I modified veristat.c locally to print prog tags, and they were
identical before and after. So I'm landing the patch as it's a nice
clean up (and it looks like we might change sha1 for sha256 soon
anyways with KP's work on signing). Thanks!

$ sudo ./veristat task_kfunc_success.bpf.o
Processing 'task_kfunc_success.bpf.o'...
TAG for task_kfunc_success.bpf.o/test_task_kfunc_flavor_relo: 3ec44141eafbb946
TAG for task_kfunc_success.bpf.o/test_task_kfunc_flavor_relo_not_found:
a2b1538278853bab
TAG for task_kfunc_success.bpf.o/test_task_acquire_release_argument:
81814696d4ca6b04
TAG for task_kfunc_success.bpf.o/test_task_acquire_release_current:
da78e4661f6fc0f5
TAG for task_kfunc_success.bpf.o/test_task_acquire_leave_in_map:
6987ab3c231b11c8
TAG for task_kfunc_success.bpf.o/test_task_xchg_release: 243afeabd9545c81
TAG for task_kfunc_success.bpf.o/test_task_map_acquire_release: 52c2aa84fd0f4163
TAG for task_kfunc_success.bpf.o/test_task_current_acquire_release:
0a49c6127a61f92e
TAG for task_kfunc_success.bpf.o/test_task_from_pid_arg: f2c0d6eb9d6f1c87
TAG for task_kfunc_success.bpf.o/test_task_from_pid_current: 3a7cc6125deb0920
TAG for task_kfunc_success.bpf.o/test_task_from_pid_invalid: a41a7678b91b0f12
TAG for task_kfunc_success.bpf.o/task_kfunc_acquire_trusted_walked:
8c38bae3eeaab179
TAG for task_kfunc_success.bpf.o/test_task_from_vpid_current: 4d5bd93fd0615005
TAG for task_kfunc_success.bpf.o/test_task_from_vpid_invalid: 87a101256cffd707
File                      Program
Verdict  Duration (us)  Insns  States  Program size  Jited size
------------------------  -------------------------------------
-------  -------------  -----  ------  ------------  ----------
task_kfunc_success.bpf.o  task_kfunc_acquire_trusted_walked
success           1111     13       1            13          73
task_kfunc_success.bpf.o  test_task_acquire_leave_in_map
success            376     73       7            57         282
task_kfunc_success.bpf.o  test_task_acquire_release_argument
success            211     18       2            41          83
task_kfunc_success.bpf.o  test_task_acquire_release_current
success            229     18       2            41          82
task_kfunc_success.bpf.o  test_task_current_acquire_release
success            212     20       2            19          96
task_kfunc_success.bpf.o  test_task_from_pid_arg
success            235     31       3            26         173
task_kfunc_success.bpf.o  test_task_from_pid_current
success            239     32       3            27         181
task_kfunc_success.bpf.o  test_task_from_pid_invalid
success            234     42       4            42         213
task_kfunc_success.bpf.o  test_task_from_vpid_current
success            139     21       2            19         106
task_kfunc_success.bpf.o  test_task_from_vpid_invalid
success            107     29       3            19         107
task_kfunc_success.bpf.o  test_task_kfunc_flavor_relo
success            270     17       1            37          88
task_kfunc_success.bpf.o  test_task_kfunc_flavor_relo_not_found
success            146      8       0            13          38
task_kfunc_success.bpf.o  test_task_map_acquire_release
success            428    126      12            95         469
task_kfunc_success.bpf.o  test_task_xchg_release
success            634    202      19           173         854
------------------------  -------------------------------------
-------  -------------  -----  ------  ------------  ----------
Done. Processed 1 files, 0 programs. Skipped 14 files, 0 programs.

> Thanks,
> Song