linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/4] perf: Remove libcrypto dependency
@ 2025-05-21 22:53 Yuzhuo Jing
  2025-05-21 22:53 ` [PATCH v1 1/4] perf utils: Add support functions for sha1 utils Yuzhuo Jing
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Yuzhuo Jing @ 2025-05-21 22:53 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Liang Kan, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt,
	Steven Rostedt (Google), James Clark, Tomas Glozar, Leo Yan,
	Guilherme Amadio, Yuzhuo Jing, Yang Jihong,
	Masami Hiramatsu (Google), Adhemerval Zanella, Wei Yang,
	Ard Biesheuvel, Mike Rapoport (Microsoft), Athira Rajeev,
	Kajol Jain, Aditya Gupta, Charlie Jenkins, Steinar H. Gunderson,
	Dr. David Alan Gilbert, Herbert Xu, Jeff Johnson, Al Viro,
	linux-kernel, linux-perf-users, llvm

Currently, genelf.c is the only file in the perf tool that depends on
libcrypto (e.g. openssl), which only uses it to calculate a SHA1/MD5
Build ID.  This patch series pulls in the SHA1 implementation from the
kernel tree, and removes the libcrypto dependency from perf.  This also
switches the default Build ID calculation method from MD5 to the more
commonly used SHA1.

Yuzhuo Jing (4):
  perf utils: Add support functions for sha1 utils
  perf tools: Add sha1 utils
  perf genelf: Remove libcrypto dependency and use sha1 utils
  tools: Remove libcrypto dependency

 tools/build/Makefile.feature            |   2 -
 tools/build/feature/Makefile            |   4 -
 tools/build/feature/test-all.c          |   5 -
 tools/build/feature/test-libcrypto.c    |  25 -----
 tools/include/linux/bitops.h            |  10 ++
 tools/include/linux/compiler.h          |  17 ++++
 tools/include/linux/string.h            |  22 +++++
 tools/perf/Documentation/perf-check.txt |   1 -
 tools/perf/Makefile.config              |  13 ---
 tools/perf/Makefile.perf                |   3 -
 tools/perf/builtin-check.c              |   1 -
 tools/perf/tests/make                   |   4 +-
 tools/perf/util/Build                   |   2 +
 tools/perf/util/genelf.c                |  72 ++------------
 tools/perf/util/sha1.c                  | 122 ++++++++++++++++++++++++
 tools/perf/util/sha1.h                  |  41 ++++++++
 tools/perf/util/sha1_base.h             | 103 ++++++++++++++++++++
 tools/perf/util/sha1_generic.c          |  49 ++++++++++
 18 files changed, 373 insertions(+), 123 deletions(-)
 delete mode 100644 tools/build/feature/test-libcrypto.c
 create mode 100644 tools/perf/util/sha1.c
 create mode 100644 tools/perf/util/sha1.h
 create mode 100644 tools/perf/util/sha1_base.h
 create mode 100644 tools/perf/util/sha1_generic.c

-- 
2.49.0.1164.gab81da1b16-goog


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v1 1/4] perf utils: Add support functions for sha1 utils
  2025-05-21 22:53 [PATCH v1 0/4] perf: Remove libcrypto dependency Yuzhuo Jing
@ 2025-05-21 22:53 ` Yuzhuo Jing
  2025-05-21 22:53 ` [PATCH v1 2/4] perf tools: Add " Yuzhuo Jing
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Yuzhuo Jing @ 2025-05-21 22:53 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Liang Kan, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt,
	Steven Rostedt (Google), James Clark, Tomas Glozar, Leo Yan,
	Guilherme Amadio, Yuzhuo Jing, Yang Jihong,
	Masami Hiramatsu (Google), Adhemerval Zanella, Wei Yang,
	Ard Biesheuvel, Mike Rapoport (Microsoft), Athira Rajeev,
	Kajol Jain, Aditya Gupta, Charlie Jenkins, Steinar H. Gunderson,
	Dr. David Alan Gilbert, Herbert Xu, Jeff Johnson, Al Viro,
	linux-kernel, linux-perf-users, llvm

Add missing functions to the shrunk down version tools headers from the
kernel headers to support sha1 utils.

Signed-off-by: Yuzhuo Jing <yuzhuo@google.com>
---
 tools/include/linux/bitops.h   | 10 ++++++++++
 tools/include/linux/compiler.h | 17 +++++++++++++++++
 tools/include/linux/string.h   | 22 ++++++++++++++++++++++
 3 files changed, 49 insertions(+)

diff --git a/tools/include/linux/bitops.h b/tools/include/linux/bitops.h
index b4e4cd071f8c..6a027031225c 100644
--- a/tools/include/linux/bitops.h
+++ b/tools/include/linux/bitops.h
@@ -89,6 +89,16 @@ static inline __u32 rol32(__u32 word, unsigned int shift)
 	return (word << shift) | (word >> ((-shift) & 31));
 }
 
+/**
+ * ror32 - rotate a 32-bit value right
+ * @word: value to rotate
+ * @shift: bits to roll
+ */
+static inline __u32 ror32(__u32 word, unsigned int shift)
+{
+	return (word >> (shift & 31)) | (word << ((-shift) & 31));
+}
+
 /**
  * sign_extend64 - sign extend a 64-bit value using specified bit as sign-bit
  * @value: value to sign extend
diff --git a/tools/include/linux/compiler.h b/tools/include/linux/compiler.h
index 9c05a59f0184..72e92b202976 100644
--- a/tools/include/linux/compiler.h
+++ b/tools/include/linux/compiler.h
@@ -40,6 +40,23 @@
 /* The "volatile" is due to gcc bugs */
 #define barrier() __asm__ __volatile__("": : :"memory")
 
+#ifndef barrier_data
+/*
+ * This version is i.e. to prevent dead stores elimination on @ptr
+ * where gcc and llvm may behave differently when otherwise using
+ * normal barrier(): while gcc behavior gets along with a normal
+ * barrier(), llvm needs an explicit input variable to be assumed
+ * clobbered. The issue is as follows: while the inline asm might
+ * access any memory it wants, the compiler could have fit all of
+ * @ptr into memory registers instead, and since @ptr never escaped
+ * from that, it proved that the inline asm wasn't touching any of
+ * it. This version works well with both compilers, i.e. we're telling
+ * the compiler that the inline asm absolutely may see the contents
+ * of @ptr. See also: https://llvm.org/bugs/show_bug.cgi?id=15495
+ */
+# define barrier_data(ptr) __asm__ __volatile__("": :"r"(ptr) :"memory")
+#endif
+
 #ifndef __always_inline
 # define __always_inline	inline __attribute__((always_inline))
 #endif
diff --git a/tools/include/linux/string.h b/tools/include/linux/string.h
index 8499f509f03e..df3c95792a51 100644
--- a/tools/include/linux/string.h
+++ b/tools/include/linux/string.h
@@ -3,6 +3,7 @@
 #define _TOOLS_LINUX_STRING_H_
 
 #include <linux/types.h>	/* for size_t */
+#include <linux/compiler.h>	/* for barrier_data */
 #include <string.h>
 
 void *memdup(const void *src, size_t len);
@@ -52,4 +53,25 @@ extern void remove_spaces(char *s);
 
 extern void *memchr_inv(const void *start, int c, size_t bytes);
 extern unsigned long long memparse(const char *ptr, char **retptr);
+
+/**
+ * memzero_explicit - Fill a region of memory (e.g. sensitive
+ *		      keying data) with 0s.
+ * @s: Pointer to the start of the area.
+ * @count: The size of the area.
+ *
+ * Note: usually using memset() is just fine (!), but in cases
+ * where clearing out _local_ data at the end of a scope is
+ * necessary, memzero_explicit() should be used instead in
+ * order to prevent the compiler from optimising away zeroing.
+ *
+ * memzero_explicit() doesn't need an arch-specific version as
+ * it just invokes the one of memset() implicitly.
+ */
+static inline void memzero_explicit(void *s, size_t count)
+{
+	memset(s, 0, count);
+	barrier_data(s);
+}
+
 #endif /* _TOOLS_LINUX_STRING_H_ */
-- 
2.49.0.1164.gab81da1b16-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 2/4] perf tools: Add sha1 utils
  2025-05-21 22:53 [PATCH v1 0/4] perf: Remove libcrypto dependency Yuzhuo Jing
  2025-05-21 22:53 ` [PATCH v1 1/4] perf utils: Add support functions for sha1 utils Yuzhuo Jing
@ 2025-05-21 22:53 ` Yuzhuo Jing
  2025-05-22 17:03   ` Arnaldo Carvalho de Melo
  2025-05-22 17:56   ` Arnaldo Carvalho de Melo
  2025-05-21 22:53 ` [PATCH v1 3/4] perf genelf: Remove libcrypto dependency and use " Yuzhuo Jing
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Yuzhuo Jing @ 2025-05-21 22:53 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Liang Kan, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt,
	Steven Rostedt (Google), James Clark, Tomas Glozar, Leo Yan,
	Guilherme Amadio, Yuzhuo Jing, Yang Jihong,
	Masami Hiramatsu (Google), Adhemerval Zanella, Wei Yang,
	Ard Biesheuvel, Mike Rapoport (Microsoft), Athira Rajeev,
	Kajol Jain, Aditya Gupta, Charlie Jenkins, Steinar H. Gunderson,
	Dr. David Alan Gilbert, Herbert Xu, Jeff Johnson, Al Viro,
	linux-kernel, linux-perf-users, llvm

Those new files are derived from the kernel tree, namely:

  tools/perf/util/sha1.c          from  lib/crypto/sha1.c
  tools/perf/util/sha1.h          from  include/crypto/sha1.h
  tools/perf/util/sha1_base.h     from  include/crypto/sha1_base.h
  tools/perf/util/sha1_generic.c  from  crypto/sha1_generic.c

The reason that we are not syncing them with the kernel tree like other
tools header files is because of the deep dependency in
include/crypto/hash.h.  It's painful to import the whole kernel crypto
driver infrastructure into tools.

The derived files get rid of struct shash_desc definition, and directly
operates on the struct sha1_state.

Signed-off-by: Yuzhuo Jing <yuzhuo@google.com>
---
 tools/perf/util/Build          |   2 +
 tools/perf/util/sha1.c         | 122 +++++++++++++++++++++++++++++++++
 tools/perf/util/sha1.h         |  41 +++++++++++
 tools/perf/util/sha1_base.h    | 103 ++++++++++++++++++++++++++++
 tools/perf/util/sha1_generic.c |  49 +++++++++++++
 5 files changed, 317 insertions(+)
 create mode 100644 tools/perf/util/sha1.c
 create mode 100644 tools/perf/util/sha1.h
 create mode 100644 tools/perf/util/sha1_base.h
 create mode 100644 tools/perf/util/sha1_generic.c

diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 7910d908c814..ecee96b3f3fa 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -41,6 +41,8 @@ perf-util-y += rbtree.o
 perf-util-y += libstring.o
 perf-util-y += bitmap.o
 perf-util-y += hweight.o
+perf-util-y += sha1.o
+perf-util-y += sha1_generic.o
 perf-util-y += smt.o
 perf-util-y += strbuf.o
 perf-util-y += string.o
diff --git a/tools/perf/util/sha1.c b/tools/perf/util/sha1.c
new file mode 100644
index 000000000000..5ae658afb56b
--- /dev/null
+++ b/tools/perf/util/sha1.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * SHA1 routine optimized to do word accesses rather than byte accesses,
+ * and to avoid unnecessary copies into the context array.
+ *
+ * This was based on the git SHA1 implementation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/bitops.h>
+#include <linux/string.h>
+#include <linux/unaligned.h>
+
+#include "sha1.h"
+
+/*
+ * If you have 32 registers or more, the compiler can (and should)
+ * try to change the array[] accesses into registers. However, on
+ * machines with less than ~25 registers, that won't really work,
+ * and at least gcc will make an unholy mess of it.
+ *
+ * So to avoid that mess which just slows things down, we force
+ * the stores to memory to actually happen (we might be better off
+ * with a 'W(t)=(val);asm("":"+m" (W(t))' there instead, as
+ * suggested by Artur Skawina - that will also make gcc unable to
+ * try to do the silly "optimize away loads" part because it won't
+ * see what the value will be).
+ *
+ * Ben Herrenschmidt reports that on PPC, the C version comes close
+ * to the optimized asm with this (ie on PPC you don't want that
+ * 'volatile', since there are lots of registers).
+ *
+ * On ARM we get the best code generation by forcing a full memory barrier
+ * between each SHA_ROUND, otherwise gcc happily get wild with spilling and
+ * the stack frame size simply explode and performance goes down the drain.
+ */
+
+#ifdef CONFIG_X86
+  #define setW(x, val) (*(volatile __u32 *)&W(x) = (val))
+#elif defined(CONFIG_ARM)
+  #define setW(x, val) do { W(x) = (val); __asm__("":::"memory"); } while (0)
+#else
+  #define setW(x, val) (W(x) = (val))
+#endif
+
+/* This "rolls" over the 512-bit array */
+#define W(x) (array[(x)&15])
+
+/*
+ * Where do we get the source from? The first 16 iterations get it from
+ * the input data, the next mix it from the 512-bit array.
+ */
+#define SHA_SRC(t) get_unaligned_be32((__u32 *)data + t)
+#define SHA_MIX(t) rol32(W(t+13) ^ W(t+8) ^ W(t+2) ^ W(t), 1)
+
+#define SHA_ROUND(t, input, fn, constant, A, B, C, D, E) do { \
+	__u32 TEMP = input(t); setW(t, TEMP); \
+	E += TEMP + rol32(A,5) + (fn) + (constant); \
+	B = ror32(B, 2); \
+	TEMP = E; E = D; D = C; C = B; B = A; A = TEMP; } while (0)
+
+#define T_0_15(t, A, B, C, D, E)  SHA_ROUND(t, SHA_SRC, (((C^D)&B)^D) , 0x5a827999, A, B, C, D, E )
+#define T_16_19(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (((C^D)&B)^D) , 0x5a827999, A, B, C, D, E )
+#define T_20_39(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (B^C^D) , 0x6ed9eba1, A, B, C, D, E )
+#define T_40_59(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, ((B&C)+(D&(B^C))) , 0x8f1bbcdc, A, B, C, D, E )
+#define T_60_79(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (B^C^D) ,  0xca62c1d6, A, B, C, D, E )
+
+/**
+ * sha1_transform - single block SHA1 transform (deprecated)
+ *
+ * @digest: 160 bit digest to update
+ * @data:   512 bits of data to hash
+ * @array:  16 words of workspace (see note)
+ *
+ * This function executes SHA-1's internal compression function.  It updates the
+ * 160-bit internal state (@digest) with a single 512-bit data block (@data).
+ *
+ * Don't use this function.  SHA-1 is no longer considered secure.  And even if
+ * you do have to use SHA-1, this isn't the correct way to hash something with
+ * SHA-1 as this doesn't handle padding and finalization.
+ *
+ * Note: If the hash is security sensitive, the caller should be sure
+ * to clear the workspace. This is left to the caller to avoid
+ * unnecessary clears between chained hashing operations.
+ */
+void sha1_transform(__u32 *digest, const char *data, __u32 *array)
+{
+	__u32 A, B, C, D, E;
+	unsigned int i = 0;
+
+	A = digest[0];
+	B = digest[1];
+	C = digest[2];
+	D = digest[3];
+	E = digest[4];
+
+	/* Round 1 - iterations 0-16 take their input from 'data' */
+	for (; i < 16; ++i)
+		T_0_15(i, A, B, C, D, E);
+
+	/* Round 1 - tail. Input from 512-bit mixing array */
+	for (; i < 20; ++i)
+		T_16_19(i, A, B, C, D, E);
+
+	/* Round 2 */
+	for (; i < 40; ++i)
+		T_20_39(i, A, B, C, D, E);
+
+	/* Round 3 */
+	for (; i < 60; ++i)
+		T_40_59(i, A, B, C, D, E);
+
+	/* Round 4 */
+	for (; i < 80; ++i)
+		T_60_79(i, A, B, C, D, E);
+
+	digest[0] += A;
+	digest[1] += B;
+	digest[2] += C;
+	digest[3] += D;
+	digest[4] += E;
+}
diff --git a/tools/perf/util/sha1.h b/tools/perf/util/sha1.h
new file mode 100644
index 000000000000..9da4ece49bc6
--- /dev/null
+++ b/tools/perf/util/sha1.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Common values for SHA-1 algorithms
+ */
+
+#ifndef _CRYPTO_SHA1_H
+#define _CRYPTO_SHA1_H
+
+#include <linux/types.h>
+
+#define SHA1_DIGEST_SIZE        20
+#define SHA1_BLOCK_SIZE         64
+
+#define SHA1_H0		0x67452301UL
+#define SHA1_H1		0xefcdab89UL
+#define SHA1_H2		0x98badcfeUL
+#define SHA1_H3		0x10325476UL
+#define SHA1_H4		0xc3d2e1f0UL
+
+struct sha1_state {
+	u32 state[SHA1_DIGEST_SIZE / 4];
+	u64 count;
+	u8 buffer[SHA1_BLOCK_SIZE];
+};
+
+extern int crypto_sha1_update(struct sha1_state *desc, const u8 *data,
+			      unsigned int len);
+
+extern int crypto_sha1_finup(struct sha1_state *desc, const u8 *data,
+			     unsigned int len, u8 *hash);
+
+/*
+ * An implementation of SHA-1's compression function.  Don't use in new code!
+ * You shouldn't be using SHA-1, and even if you *have* to use SHA-1, this isn't
+ * the correct way to hash something with SHA-1 (use crypto_shash instead).
+ */
+#define SHA1_DIGEST_WORDS	(SHA1_DIGEST_SIZE / 4)
+#define SHA1_WORKSPACE_WORDS	16
+void sha1_transform(__u32 *digest, const char *data, __u32 *W);
+
+#endif /* _CRYPTO_SHA1_H */
diff --git a/tools/perf/util/sha1_base.h b/tools/perf/util/sha1_base.h
new file mode 100644
index 000000000000..cea22c5a4952
--- /dev/null
+++ b/tools/perf/util/sha1_base.h
@@ -0,0 +1,103 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * sha1_base.h - core logic for SHA-1 implementations
+ *
+ * Copyright (C) 2015 Linaro Ltd <ard.biesheuvel@linaro.org>
+ */
+
+#ifndef _CRYPTO_SHA1_BASE_H
+#define _CRYPTO_SHA1_BASE_H
+
+#include <linux/string.h>
+
+#include <linux/kernel.h>
+#include <linux/unaligned.h>
+
+#include "sha1.h"
+
+typedef void (sha1_block_fn)(struct sha1_state *sst, u8 const *src, int blocks);
+
+static inline int sha1_base_init(struct sha1_state *sctx)
+{
+	sctx->state[0] = SHA1_H0;
+	sctx->state[1] = SHA1_H1;
+	sctx->state[2] = SHA1_H2;
+	sctx->state[3] = SHA1_H3;
+	sctx->state[4] = SHA1_H4;
+	sctx->count = 0;
+
+	return 0;
+}
+
+static inline int sha1_base_do_update(struct sha1_state *sctx,
+				      const u8 *data,
+				      unsigned int len,
+				      sha1_block_fn *block_fn)
+{
+	unsigned int partial = sctx->count % SHA1_BLOCK_SIZE;
+
+	sctx->count += len;
+
+	if (unlikely((partial + len) >= SHA1_BLOCK_SIZE)) {
+		int blocks;
+
+		if (partial) {
+			int p = SHA1_BLOCK_SIZE - partial;
+
+			memcpy(sctx->buffer + partial, data, p);
+			data += p;
+			len -= p;
+
+			block_fn(sctx, sctx->buffer, 1);
+		}
+
+		blocks = len / SHA1_BLOCK_SIZE;
+		len %= SHA1_BLOCK_SIZE;
+
+		if (blocks) {
+			block_fn(sctx, data, blocks);
+			data += blocks * SHA1_BLOCK_SIZE;
+		}
+		partial = 0;
+	}
+	if (len)
+		memcpy(sctx->buffer + partial, data, len);
+
+	return 0;
+}
+
+static inline int sha1_base_do_finalize(struct sha1_state *sctx,
+					sha1_block_fn *block_fn)
+{
+	const int bit_offset = SHA1_BLOCK_SIZE - sizeof(__be64);
+	__be64 *bits = (__be64 *)(sctx->buffer + bit_offset);
+	unsigned int partial = sctx->count % SHA1_BLOCK_SIZE;
+
+	sctx->buffer[partial++] = 0x80;
+	if (partial > bit_offset) {
+		memset(sctx->buffer + partial, 0x0, SHA1_BLOCK_SIZE - partial);
+		partial = 0;
+
+		block_fn(sctx, sctx->buffer, 1);
+	}
+
+	memset(sctx->buffer + partial, 0x0, bit_offset - partial);
+	*bits = cpu_to_be64(sctx->count << 3);
+	block_fn(sctx, sctx->buffer, 1);
+
+	return 0;
+}
+
+static inline int sha1_base_finish(struct sha1_state *sctx, u8 *out)
+{
+	__be32 *digest = (__be32 *)out;
+	int i;
+
+	for (i = 0; i < SHA1_DIGEST_SIZE / (int)sizeof(__be32); i++)
+		put_unaligned_be32(sctx->state[i], digest++);
+
+	memzero_explicit(sctx, sizeof(*sctx));
+	return 0;
+}
+
+#endif /* _CRYPTO_SHA1_BASE_H */
diff --git a/tools/perf/util/sha1_generic.c b/tools/perf/util/sha1_generic.c
new file mode 100644
index 000000000000..b0a7af370d59
--- /dev/null
+++ b/tools/perf/util/sha1_generic.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Cryptographic API.
+ *
+ * SHA1 Secure Hash Algorithm.
+ *
+ * Derived from cryptoapi implementation, adapted for in-place
+ * scatterlist interface.
+ *
+ * Copyright (c) Alan Smithee.
+ * Copyright (c) Andrew McDonald <andrew@mcdonald.org.uk>
+ * Copyright (c) Jean-Francois Dive <jef@linuxbe.org>
+ */
+#include <linux/types.h>
+#include <linux/string.h>
+#include <asm/byteorder.h>
+
+#include "sha1_base.h"
+
+static void sha1_generic_block_fn(struct sha1_state *sst, u8 const *src,
+				  int blocks)
+{
+	u32 temp[SHA1_WORKSPACE_WORDS];
+
+	while (blocks--) {
+		sha1_transform(sst->state, (const char *)src, temp);
+		src += SHA1_BLOCK_SIZE;
+	}
+	memzero_explicit(temp, sizeof(temp));
+}
+
+int crypto_sha1_update(struct sha1_state *desc, const u8 *data,
+		       unsigned int len)
+{
+	return sha1_base_do_update(desc, data, len, sha1_generic_block_fn);
+}
+
+static int sha1_final(struct sha1_state *desc, u8 *out)
+{
+	sha1_base_do_finalize(desc, sha1_generic_block_fn);
+	return sha1_base_finish(desc, out);
+}
+
+int crypto_sha1_finup(struct sha1_state *desc, const u8 *data,
+		      unsigned int len, u8 *out)
+{
+	sha1_base_do_update(desc, data, len, sha1_generic_block_fn);
+	return sha1_final(desc, out);
+}
-- 
2.49.0.1164.gab81da1b16-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 3/4] perf genelf: Remove libcrypto dependency and use sha1 utils
  2025-05-21 22:53 [PATCH v1 0/4] perf: Remove libcrypto dependency Yuzhuo Jing
  2025-05-21 22:53 ` [PATCH v1 1/4] perf utils: Add support functions for sha1 utils Yuzhuo Jing
  2025-05-21 22:53 ` [PATCH v1 2/4] perf tools: Add " Yuzhuo Jing
@ 2025-05-21 22:53 ` Yuzhuo Jing
  2025-05-22 17:05   ` Arnaldo Carvalho de Melo
  2025-05-21 22:53 ` [PATCH v1 4/4] tools: Remove libcrypto dependency Yuzhuo Jing
  2025-05-29 19:31 ` [PATCH v1 0/4] perf: " Ian Rogers
  4 siblings, 1 reply; 15+ messages in thread
From: Yuzhuo Jing @ 2025-05-21 22:53 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Liang Kan, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt,
	Steven Rostedt (Google), James Clark, Tomas Glozar, Leo Yan,
	Guilherme Amadio, Yuzhuo Jing, Yang Jihong,
	Masami Hiramatsu (Google), Adhemerval Zanella, Wei Yang,
	Ard Biesheuvel, Mike Rapoport (Microsoft), Athira Rajeev,
	Kajol Jain, Aditya Gupta, Charlie Jenkins, Steinar H. Gunderson,
	Dr. David Alan Gilbert, Herbert Xu, Jeff Johnson, Al Viro,
	linux-kernel, linux-perf-users, llvm

genelf is the only file in perf that depends on libcrypto (or openssl)
which only calculates a Build ID (SHA1, MD5, or URANDOM).  SHA1 was
expected to be the default option, but MD5 was used by default due to
previous issues when linking against Java.  This commit switches genelf
to use in-house SHA1 utils, and also removes MD5 and URANDOM options
since we have a reliable SHA1 implementation to rely on.  It passes the
tools/perf/tests/shell/test_java_symbol.sh test.

Signed-off-by: Yuzhuo Jing <yuzhuo@google.com>
---
 tools/perf/util/genelf.c | 72 ++++------------------------------------
 1 file changed, 6 insertions(+), 66 deletions(-)

diff --git a/tools/perf/util/genelf.c b/tools/perf/util/genelf.c
index cdce7f173d00..cfedb29260ef 100644
--- a/tools/perf/util/genelf.c
+++ b/tools/perf/util/genelf.c
@@ -28,24 +28,7 @@
 #define NT_GNU_BUILD_ID 3
 #endif
 
-#define BUILD_ID_URANDOM /* different uuid for each run */
-
-#ifdef HAVE_LIBCRYPTO_SUPPORT
-
-#define BUILD_ID_MD5
-#undef BUILD_ID_SHA	/* does not seem to work well when linked with Java */
-#undef BUILD_ID_URANDOM /* different uuid for each run */
-
-#ifdef BUILD_ID_SHA
-#include <openssl/sha.h>
-#endif
-
-#ifdef BUILD_ID_MD5
-#include <openssl/evp.h>
-#include <openssl/md5.h>
-#endif
-#endif
-
+#include "sha1_base.h"
 
 typedef struct {
   unsigned int namesz;  /* Size of entry's owner string */
@@ -92,64 +75,21 @@ static Elf_Sym symtab[]={
 	}
 };
 
-#ifdef BUILD_ID_URANDOM
-static void
-gen_build_id(struct buildid_note *note,
-	     unsigned long load_addr __maybe_unused,
-	     const void *code __maybe_unused,
-	     size_t csize __maybe_unused)
-{
-	int fd;
-	size_t sz = sizeof(note->build_id);
-	ssize_t sret;
-
-	fd = open("/dev/urandom", O_RDONLY);
-	if (fd == -1)
-		err(1, "cannot access /dev/urandom for buildid");
-
-	sret = read(fd, note->build_id, sz);
-
-	close(fd);
-
-	if (sret != (ssize_t)sz)
-		memset(note->build_id, 0, sz);
-}
-#endif
-
-#ifdef BUILD_ID_SHA
 static void
 gen_build_id(struct buildid_note *note,
 	     unsigned long load_addr __maybe_unused,
 	     const void *code,
 	     size_t csize)
 {
-	if (sizeof(note->build_id) < SHA_DIGEST_LENGTH)
-		errx(1, "build_id too small for SHA1");
-
-	SHA1(code, csize, (unsigned char *)note->build_id);
-}
-#endif
-
-#ifdef BUILD_ID_MD5
-static void
-gen_build_id(struct buildid_note *note, unsigned long load_addr, const void *code, size_t csize)
-{
-	EVP_MD_CTX *mdctx;
+	struct sha1_state sctx;
 
-	if (sizeof(note->build_id) < 16)
-		errx(1, "build_id too small for MD5");
+	if (sizeof(note->build_id) < SHA1_DIGEST_SIZE)
+		errx(1, "build_id too small for SHA1");
 
-	mdctx = EVP_MD_CTX_new();
-	if (!mdctx)
-		errx(2, "failed to create EVP_MD_CTX");
+	sha1_base_init(&sctx);
 
-	EVP_DigestInit_ex(mdctx, EVP_md5(), NULL);
-	EVP_DigestUpdate(mdctx, &load_addr, sizeof(load_addr));
-	EVP_DigestUpdate(mdctx, code, csize);
-	EVP_DigestFinal_ex(mdctx, (unsigned char *)note->build_id, NULL);
-	EVP_MD_CTX_free(mdctx);
+	crypto_sha1_finup(&sctx, code, csize, (unsigned char *)note->build_id);
 }
-#endif
 
 static int
 jit_add_eh_frame_info(Elf *e, void* unwinding, uint64_t unwinding_header_size,
-- 
2.49.0.1164.gab81da1b16-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 4/4] tools: Remove libcrypto dependency
  2025-05-21 22:53 [PATCH v1 0/4] perf: Remove libcrypto dependency Yuzhuo Jing
                   ` (2 preceding siblings ...)
  2025-05-21 22:53 ` [PATCH v1 3/4] perf genelf: Remove libcrypto dependency and use " Yuzhuo Jing
@ 2025-05-21 22:53 ` Yuzhuo Jing
  2025-05-22 17:30   ` Arnaldo Carvalho de Melo
  2025-05-29 19:31 ` [PATCH v1 0/4] perf: " Ian Rogers
  4 siblings, 1 reply; 15+ messages in thread
From: Yuzhuo Jing @ 2025-05-21 22:53 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Liang Kan, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt,
	Steven Rostedt (Google), James Clark, Tomas Glozar, Leo Yan,
	Guilherme Amadio, Yuzhuo Jing, Yang Jihong,
	Masami Hiramatsu (Google), Adhemerval Zanella, Wei Yang,
	Ard Biesheuvel, Mike Rapoport (Microsoft), Athira Rajeev,
	Kajol Jain, Aditya Gupta, Charlie Jenkins, Steinar H. Gunderson,
	Dr. David Alan Gilbert, Herbert Xu, Jeff Johnson, Al Viro,
	linux-kernel, linux-perf-users, llvm

Remove all occurrence of libcrypto in the build system.

Signed-off-by: Yuzhuo Jing <yuzhuo@google.com>
---
 tools/build/Makefile.feature            |  2 --
 tools/build/feature/Makefile            |  4 ----
 tools/build/feature/test-all.c          |  5 -----
 tools/build/feature/test-libcrypto.c    | 25 -------------------------
 tools/perf/Documentation/perf-check.txt |  1 -
 tools/perf/Makefile.config              | 13 -------------
 tools/perf/Makefile.perf                |  3 ---
 tools/perf/builtin-check.c              |  1 -
 tools/perf/tests/make                   |  4 +---
 9 files changed, 1 insertion(+), 57 deletions(-)
 delete mode 100644 tools/build/feature/test-libcrypto.c

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 57bd995ce6af..bbadfb5568eb 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -86,7 +86,6 @@ FEATURE_TESTS_BASIC :=                  \
         libtraceevent                   \
         libtracefs                      \
         libcpupower                     \
-        libcrypto                       \
         pthread-attr-setaffinity-np     \
         pthread-barrier     		\
         reallocarray                    \
@@ -152,7 +151,6 @@ FEATURE_DISPLAY ?=              \
          numa_num_possible_cpus \
          libperl                \
          libpython              \
-         libcrypto              \
          libcapstone            \
          llvm-perf              \
          zlib                   \
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index b8b5fb183dd4..04a4aa0341aa 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -38,7 +38,6 @@ FILES=                                          \
          test-libtraceevent.bin                 \
          test-libcpupower.bin                   \
          test-libtracefs.bin                    \
-         test-libcrypto.bin                     \
          test-libunwind.bin                     \
          test-libunwind-debug-frame.bin         \
          test-libunwind-x86.bin                 \
@@ -246,9 +245,6 @@ $(OUTPUT)test-libcpupower.bin:
 $(OUTPUT)test-libtracefs.bin:
 	 $(BUILD) $(shell $(PKG_CONFIG) --cflags libtracefs 2>/dev/null) -ltracefs
 
-$(OUTPUT)test-libcrypto.bin:
-	$(BUILD) -lcrypto
-
 $(OUTPUT)test-gtk2.bin:
 	$(BUILD) $(shell $(PKG_CONFIG) --libs --cflags gtk+-2.0 2>/dev/null) -Wno-deprecated-declarations
 
diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
index 03ddaac6f4c4..ce72e2061ac0 100644
--- a/tools/build/feature/test-all.c
+++ b/tools/build/feature/test-all.c
@@ -138,10 +138,6 @@
 # include "test-bpf.c"
 #undef main
 
-#define main main_test_libcrypto
-# include "test-libcrypto.c"
-#undef main
-
 #define main main_test_sdt
 # include "test-sdt.c"
 #undef main
@@ -206,7 +202,6 @@ int main(int argc, char *argv[])
 	main_test_lzma();
 	main_test_get_cpuid();
 	main_test_bpf();
-	main_test_libcrypto();
 	main_test_scandirat();
 	main_test_sched_getcpu();
 	main_test_sdt();
diff --git a/tools/build/feature/test-libcrypto.c b/tools/build/feature/test-libcrypto.c
deleted file mode 100644
index bc34a5bbb504..000000000000
--- a/tools/build/feature/test-libcrypto.c
+++ /dev/null
@@ -1,25 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#include <openssl/evp.h>
-#include <openssl/sha.h>
-#include <openssl/md5.h>
-
-int main(void)
-{
-	EVP_MD_CTX *mdctx;
-	unsigned char md[MD5_DIGEST_LENGTH + SHA_DIGEST_LENGTH];
-	unsigned char dat[] = "12345";
-	unsigned int digest_len;
-
-	mdctx = EVP_MD_CTX_new();
-	if (!mdctx)
-		return 0;
-
-	EVP_DigestInit_ex(mdctx, EVP_md5(), NULL);
-	EVP_DigestUpdate(mdctx, &dat[0], sizeof(dat));
-	EVP_DigestFinal_ex(mdctx, &md[0], &digest_len);
-	EVP_MD_CTX_free(mdctx);
-
-	SHA1(&dat[0], sizeof(dat), &md[0]);
-
-	return 0;
-}
diff --git a/tools/perf/Documentation/perf-check.txt b/tools/perf/Documentation/perf-check.txt
index a764a4629220..2b96cb578658 100644
--- a/tools/perf/Documentation/perf-check.txt
+++ b/tools/perf/Documentation/perf-check.txt
@@ -53,7 +53,6 @@ feature::
                 auxtrace                /  HAVE_AUXTRACE_SUPPORT
                 libbfd                  /  HAVE_LIBBFD_SUPPORT
                 libcapstone             /  HAVE_LIBCAPSTONE_SUPPORT
-                libcrypto               /  HAVE_LIBCRYPTO_SUPPORT
                 libdw-dwarf-unwind      /  HAVE_LIBDW_SUPPORT
                 libelf                  /  HAVE_LIBELF_SUPPORT
                 libnuma                 /  HAVE_LIBNUMA_SUPPORT
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index d1ea7bf44964..d19d1f132726 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -130,8 +130,6 @@ ifndef NO_LIBUNWIND
   FEATURE_CHECK_LDFLAGS-libunwind-x86_64 += -lunwind -llzma -lunwind-x86_64
 endif
 
-FEATURE_CHECK_LDFLAGS-libcrypto = -lcrypto
-
 ifdef CSINCLUDES
   LIBOPENCSD_CFLAGS := -I$(CSINCLUDES)
 endif
@@ -772,17 +770,6 @@ ifneq ($(NO_LIBTRACEEVENT),1)
   $(call detected,CONFIG_TRACE)
 endif
 
-ifndef NO_LIBCRYPTO
-  ifneq ($(feature-libcrypto), 1)
-    $(warning No libcrypto.h found, disables jitted code injection, please install openssl-devel or libssl-dev)
-    NO_LIBCRYPTO := 1
-  else
-    CFLAGS += -DHAVE_LIBCRYPTO_SUPPORT
-    EXTLIBS += -lcrypto
-    $(call detected,CONFIG_CRYPTO)
-  endif
-endif
-
 ifndef NO_SLANG
   ifneq ($(feature-libslang), 1)
     ifneq ($(feature-libslang-include-subdir), 1)
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index d4c7031b01a7..9246c94656e0 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -61,9 +61,6 @@ include ../scripts/utilities.mak
 #
 # Define NO_LIBBIONIC if you do not want bionic support
 #
-# Define NO_LIBCRYPTO if you do not want libcrypto (openssl) support
-# used for generating build-ids for ELFs generated by jitdump.
-#
 # Define NO_LIBDW_DWARF_UNWIND if you do not want libdw support
 # for dwarf backtrace post unwind.
 #
diff --git a/tools/perf/builtin-check.c b/tools/perf/builtin-check.c
index 9a509cb3bb9a..ad49f2564aae 100644
--- a/tools/perf/builtin-check.c
+++ b/tools/perf/builtin-check.c
@@ -44,7 +44,6 @@ struct feature_status supported_features[] = {
 	FEATURE_STATUS("auxtrace", HAVE_AUXTRACE_SUPPORT),
 	FEATURE_STATUS_TIP("libbfd", HAVE_LIBBFD_SUPPORT, "Deprecated, license incompatibility, use BUILD_NONDISTRO=1 and install binutils-dev[el]"),
 	FEATURE_STATUS("libcapstone", HAVE_LIBCAPSTONE_SUPPORT),
-	FEATURE_STATUS("libcrypto", HAVE_LIBCRYPTO_SUPPORT),
 	FEATURE_STATUS("libdw-dwarf-unwind", HAVE_LIBDW_SUPPORT),
 	FEATURE_STATUS("libelf", HAVE_LIBELF_SUPPORT),
 	FEATURE_STATUS("libnuma", HAVE_LIBNUMA_SUPPORT),
diff --git a/tools/perf/tests/make b/tools/perf/tests/make
index 0ee94caf9ec1..e3651e5b195a 100644
--- a/tools/perf/tests/make
+++ b/tools/perf/tests/make
@@ -91,7 +91,6 @@ make_no_auxtrace    := NO_AUXTRACE=1
 make_no_libbpf	    := NO_LIBBPF=1
 make_libbpf_dynamic := LIBBPF_DYNAMIC=1
 make_no_libbpf_DEBUG := NO_LIBBPF=1 DEBUG=1
-make_no_libcrypto   := NO_LIBCRYPTO=1
 make_no_libllvm     := NO_LIBLLVM=1
 make_with_babeltrace:= LIBBABELTRACE=1
 make_with_coresight := CORESIGHT=1
@@ -122,7 +121,7 @@ make_minimal        := NO_LIBPERL=1 NO_LIBPYTHON=1 NO_GTK2=1
 make_minimal        += NO_DEMANGLE=1 NO_LIBELF=1 NO_BACKTRACE=1
 make_minimal        += NO_LIBNUMA=1 NO_LIBBIONIC=1
 make_minimal        += NO_LIBDW_DWARF_UNWIND=1 NO_AUXTRACE=1 NO_LIBBPF=1
-make_minimal        += NO_LIBCRYPTO=1 NO_SDT=1 NO_JVMTI=1 NO_LIBZSTD=1
+make_minimal        += NO_SDT=1 NO_JVMTI=1 NO_LIBZSTD=1
 make_minimal        += NO_LIBCAP=1 NO_CAPSTONE=1
 
 # $(run) contains all available tests
@@ -160,7 +159,6 @@ run += make_no_libbionic
 run += make_no_auxtrace
 run += make_no_libbpf
 run += make_no_libbpf_DEBUG
-run += make_no_libcrypto
 run += make_no_libllvm
 run += make_no_sdt
 run += make_no_syscall_tbl
-- 
2.49.0.1164.gab81da1b16-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 2/4] perf tools: Add sha1 utils
  2025-05-21 22:53 ` [PATCH v1 2/4] perf tools: Add " Yuzhuo Jing
@ 2025-05-22 17:03   ` Arnaldo Carvalho de Melo
  2025-05-22 17:56   ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-22 17:03 UTC (permalink / raw)
  To: Yuzhuo Jing
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	Liang Kan, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
	Justin Stitt, Steven Rostedt (Google), James Clark, Tomas Glozar,
	Leo Yan, Guilherme Amadio, Yang Jihong, Masami Hiramatsu (Google),
	Adhemerval Zanella, Wei Yang, Ard Biesheuvel,
	Mike Rapoport (Microsoft), Athira Rajeev, Kajol Jain,
	Aditya Gupta, Charlie Jenkins, Steinar H. Gunderson,
	Dr. David Alan Gilbert, Herbert Xu, Jeff Johnson, Al Viro,
	linux-kernel, linux-perf-users, llvm

On Wed, May 21, 2025 at 03:53:05PM -0700, Yuzhuo Jing wrote:
> Those new files are derived from the kernel tree, namely:
> 
>   tools/perf/util/sha1.c          from  lib/crypto/sha1.c
>   tools/perf/util/sha1.h          from  include/crypto/sha1.h
>   tools/perf/util/sha1_base.h     from  include/crypto/sha1_base.h
>   tools/perf/util/sha1_generic.c  from  crypto/sha1_generic.c
> 
> The reason that we are not syncing them with the kernel tree like other
> tools header files is because of the deep dependency in

That is ok, we do it in some other cases, but while looking at this
patchset I checked and the source code for sha1_transform() is verbatim
the same, as intended, I wonder if we could add infrastructure to
check_headers.h to instead of checking whole files, check if the source
code we got from the kernel is the same, something along the lines of:

⬢ [acme@toolbx perf-tools-next]$ line=$(ctags -x --c-kinds=f lib/crypto/sha1.c | awk '$1 == "sha1_transform" {print $3}')
⬢ [acme@toolbx perf-tools-next]$ sed -n $line,\$p lib/crypto/sha1.c | awk '{print} /\{/ {c++} /\}/ {c--; if (c==0) exit}'
void sha1_transform(__u32 *digest, const char *data, __u32 *array)
{
	__u32 A, B, C, D, E;
	unsigned int i = 0;

	A = digest[0];
	B = digest[1];
	C = digest[2];
	D = digest[3];
	E = digest[4];

	/* Round 1 - iterations 0-16 take their input from 'data' */
	for (; i < 16; ++i)
		T_0_15(i, A, B, C, D, E);

	/* Round 1 - tail. Input from 512-bit mixing array */
	for (; i < 20; ++i)
		T_16_19(i, A, B, C, D, E);

	/* Round 2 */
	for (; i < 40; ++i)
		T_20_39(i, A, B, C, D, E);

	/* Round 3 */
	for (; i < 60; ++i)
		T_40_59(i, A, B, C, D, E);

	/* Round 4 */
	for (; i < 80; ++i)
		T_60_79(i, A, B, C, D, E);

	digest[0] += A;
	digest[1] += B;
	digest[2] += C;
	digest[3] += D;
	digest[4] += E;
}
⬢ [acme@toolbx perf-tools-next]$

But that can be done later :-)

- Arnaldo

> include/crypto/hash.h.  It's painful to import the whole kernel crypto
> driver infrastructure into tools.
 
> The derived files get rid of struct shash_desc definition, and directly
> operates on the struct sha1_state.
 
> Signed-off-by: Yuzhuo Jing <yuzhuo@google.com>
> ---
>  tools/perf/util/Build          |   2 +
>  tools/perf/util/sha1.c         | 122 +++++++++++++++++++++++++++++++++
>  tools/perf/util/sha1.h         |  41 +++++++++++
>  tools/perf/util/sha1_base.h    | 103 ++++++++++++++++++++++++++++
>  tools/perf/util/sha1_generic.c |  49 +++++++++++++
>  5 files changed, 317 insertions(+)
>  create mode 100644 tools/perf/util/sha1.c
>  create mode 100644 tools/perf/util/sha1.h
>  create mode 100644 tools/perf/util/sha1_base.h
>  create mode 100644 tools/perf/util/sha1_generic.c
> 
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 7910d908c814..ecee96b3f3fa 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -41,6 +41,8 @@ perf-util-y += rbtree.o
>  perf-util-y += libstring.o
>  perf-util-y += bitmap.o
>  perf-util-y += hweight.o
> +perf-util-y += sha1.o
> +perf-util-y += sha1_generic.o
>  perf-util-y += smt.o
>  perf-util-y += strbuf.o
>  perf-util-y += string.o
> diff --git a/tools/perf/util/sha1.c b/tools/perf/util/sha1.c
> new file mode 100644
> index 000000000000..5ae658afb56b
> --- /dev/null
> +++ b/tools/perf/util/sha1.c
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SHA1 routine optimized to do word accesses rather than byte accesses,
> + * and to avoid unnecessary copies into the context array.
> + *
> + * This was based on the git SHA1 implementation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/bitops.h>
> +#include <linux/string.h>
> +#include <linux/unaligned.h>
> +
> +#include "sha1.h"
> +
> +/*
> + * If you have 32 registers or more, the compiler can (and should)
> + * try to change the array[] accesses into registers. However, on
> + * machines with less than ~25 registers, that won't really work,
> + * and at least gcc will make an unholy mess of it.
> + *
> + * So to avoid that mess which just slows things down, we force
> + * the stores to memory to actually happen (we might be better off
> + * with a 'W(t)=(val);asm("":"+m" (W(t))' there instead, as
> + * suggested by Artur Skawina - that will also make gcc unable to
> + * try to do the silly "optimize away loads" part because it won't
> + * see what the value will be).
> + *
> + * Ben Herrenschmidt reports that on PPC, the C version comes close
> + * to the optimized asm with this (ie on PPC you don't want that
> + * 'volatile', since there are lots of registers).
> + *
> + * On ARM we get the best code generation by forcing a full memory barrier
> + * between each SHA_ROUND, otherwise gcc happily get wild with spilling and
> + * the stack frame size simply explode and performance goes down the drain.
> + */
> +
> +#ifdef CONFIG_X86
> +  #define setW(x, val) (*(volatile __u32 *)&W(x) = (val))
> +#elif defined(CONFIG_ARM)
> +  #define setW(x, val) do { W(x) = (val); __asm__("":::"memory"); } while (0)
> +#else
> +  #define setW(x, val) (W(x) = (val))
> +#endif
> +
> +/* This "rolls" over the 512-bit array */
> +#define W(x) (array[(x)&15])
> +
> +/*
> + * Where do we get the source from? The first 16 iterations get it from
> + * the input data, the next mix it from the 512-bit array.
> + */
> +#define SHA_SRC(t) get_unaligned_be32((__u32 *)data + t)
> +#define SHA_MIX(t) rol32(W(t+13) ^ W(t+8) ^ W(t+2) ^ W(t), 1)
> +
> +#define SHA_ROUND(t, input, fn, constant, A, B, C, D, E) do { \
> +	__u32 TEMP = input(t); setW(t, TEMP); \
> +	E += TEMP + rol32(A,5) + (fn) + (constant); \
> +	B = ror32(B, 2); \
> +	TEMP = E; E = D; D = C; C = B; B = A; A = TEMP; } while (0)
> +
> +#define T_0_15(t, A, B, C, D, E)  SHA_ROUND(t, SHA_SRC, (((C^D)&B)^D) , 0x5a827999, A, B, C, D, E )
> +#define T_16_19(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (((C^D)&B)^D) , 0x5a827999, A, B, C, D, E )
> +#define T_20_39(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (B^C^D) , 0x6ed9eba1, A, B, C, D, E )
> +#define T_40_59(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, ((B&C)+(D&(B^C))) , 0x8f1bbcdc, A, B, C, D, E )
> +#define T_60_79(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (B^C^D) ,  0xca62c1d6, A, B, C, D, E )
> +
> +/**
> + * sha1_transform - single block SHA1 transform (deprecated)
> + *
> + * @digest: 160 bit digest to update
> + * @data:   512 bits of data to hash
> + * @array:  16 words of workspace (see note)
> + *
> + * This function executes SHA-1's internal compression function.  It updates the
> + * 160-bit internal state (@digest) with a single 512-bit data block (@data).
> + *
> + * Don't use this function.  SHA-1 is no longer considered secure.  And even if
> + * you do have to use SHA-1, this isn't the correct way to hash something with
> + * SHA-1 as this doesn't handle padding and finalization.
> + *
> + * Note: If the hash is security sensitive, the caller should be sure
> + * to clear the workspace. This is left to the caller to avoid
> + * unnecessary clears between chained hashing operations.
> + */
> +void sha1_transform(__u32 *digest, const char *data, __u32 *array)
> +{
> +	__u32 A, B, C, D, E;
> +	unsigned int i = 0;
> +
> +	A = digest[0];
> +	B = digest[1];
> +	C = digest[2];
> +	D = digest[3];
> +	E = digest[4];
> +
> +	/* Round 1 - iterations 0-16 take their input from 'data' */
> +	for (; i < 16; ++i)
> +		T_0_15(i, A, B, C, D, E);
> +
> +	/* Round 1 - tail. Input from 512-bit mixing array */
> +	for (; i < 20; ++i)
> +		T_16_19(i, A, B, C, D, E);
> +
> +	/* Round 2 */
> +	for (; i < 40; ++i)
> +		T_20_39(i, A, B, C, D, E);
> +
> +	/* Round 3 */
> +	for (; i < 60; ++i)
> +		T_40_59(i, A, B, C, D, E);
> +
> +	/* Round 4 */
> +	for (; i < 80; ++i)
> +		T_60_79(i, A, B, C, D, E);
> +
> +	digest[0] += A;
> +	digest[1] += B;
> +	digest[2] += C;
> +	digest[3] += D;
> +	digest[4] += E;
> +}
> diff --git a/tools/perf/util/sha1.h b/tools/perf/util/sha1.h
> new file mode 100644
> index 000000000000..9da4ece49bc6
> --- /dev/null
> +++ b/tools/perf/util/sha1.h
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Common values for SHA-1 algorithms
> + */
> +
> +#ifndef _CRYPTO_SHA1_H
> +#define _CRYPTO_SHA1_H
> +
> +#include <linux/types.h>
> +
> +#define SHA1_DIGEST_SIZE        20
> +#define SHA1_BLOCK_SIZE         64
> +
> +#define SHA1_H0		0x67452301UL
> +#define SHA1_H1		0xefcdab89UL
> +#define SHA1_H2		0x98badcfeUL
> +#define SHA1_H3		0x10325476UL
> +#define SHA1_H4		0xc3d2e1f0UL
> +
> +struct sha1_state {
> +	u32 state[SHA1_DIGEST_SIZE / 4];
> +	u64 count;
> +	u8 buffer[SHA1_BLOCK_SIZE];
> +};
> +
> +extern int crypto_sha1_update(struct sha1_state *desc, const u8 *data,
> +			      unsigned int len);
> +
> +extern int crypto_sha1_finup(struct sha1_state *desc, const u8 *data,
> +			     unsigned int len, u8 *hash);
> +
> +/*
> + * An implementation of SHA-1's compression function.  Don't use in new code!
> + * You shouldn't be using SHA-1, and even if you *have* to use SHA-1, this isn't
> + * the correct way to hash something with SHA-1 (use crypto_shash instead).
> + */
> +#define SHA1_DIGEST_WORDS	(SHA1_DIGEST_SIZE / 4)
> +#define SHA1_WORKSPACE_WORDS	16
> +void sha1_transform(__u32 *digest, const char *data, __u32 *W);
> +
> +#endif /* _CRYPTO_SHA1_H */
> diff --git a/tools/perf/util/sha1_base.h b/tools/perf/util/sha1_base.h
> new file mode 100644
> index 000000000000..cea22c5a4952
> --- /dev/null
> +++ b/tools/perf/util/sha1_base.h
> @@ -0,0 +1,103 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * sha1_base.h - core logic for SHA-1 implementations
> + *
> + * Copyright (C) 2015 Linaro Ltd <ard.biesheuvel@linaro.org>
> + */
> +
> +#ifndef _CRYPTO_SHA1_BASE_H
> +#define _CRYPTO_SHA1_BASE_H
> +
> +#include <linux/string.h>
> +
> +#include <linux/kernel.h>
> +#include <linux/unaligned.h>
> +
> +#include "sha1.h"
> +
> +typedef void (sha1_block_fn)(struct sha1_state *sst, u8 const *src, int blocks);
> +
> +static inline int sha1_base_init(struct sha1_state *sctx)
> +{
> +	sctx->state[0] = SHA1_H0;
> +	sctx->state[1] = SHA1_H1;
> +	sctx->state[2] = SHA1_H2;
> +	sctx->state[3] = SHA1_H3;
> +	sctx->state[4] = SHA1_H4;
> +	sctx->count = 0;
> +
> +	return 0;
> +}
> +
> +static inline int sha1_base_do_update(struct sha1_state *sctx,
> +				      const u8 *data,
> +				      unsigned int len,
> +				      sha1_block_fn *block_fn)
> +{
> +	unsigned int partial = sctx->count % SHA1_BLOCK_SIZE;
> +
> +	sctx->count += len;
> +
> +	if (unlikely((partial + len) >= SHA1_BLOCK_SIZE)) {
> +		int blocks;
> +
> +		if (partial) {
> +			int p = SHA1_BLOCK_SIZE - partial;
> +
> +			memcpy(sctx->buffer + partial, data, p);
> +			data += p;
> +			len -= p;
> +
> +			block_fn(sctx, sctx->buffer, 1);
> +		}
> +
> +		blocks = len / SHA1_BLOCK_SIZE;
> +		len %= SHA1_BLOCK_SIZE;
> +
> +		if (blocks) {
> +			block_fn(sctx, data, blocks);
> +			data += blocks * SHA1_BLOCK_SIZE;
> +		}
> +		partial = 0;
> +	}
> +	if (len)
> +		memcpy(sctx->buffer + partial, data, len);
> +
> +	return 0;
> +}
> +
> +static inline int sha1_base_do_finalize(struct sha1_state *sctx,
> +					sha1_block_fn *block_fn)
> +{
> +	const int bit_offset = SHA1_BLOCK_SIZE - sizeof(__be64);
> +	__be64 *bits = (__be64 *)(sctx->buffer + bit_offset);
> +	unsigned int partial = sctx->count % SHA1_BLOCK_SIZE;
> +
> +	sctx->buffer[partial++] = 0x80;
> +	if (partial > bit_offset) {
> +		memset(sctx->buffer + partial, 0x0, SHA1_BLOCK_SIZE - partial);
> +		partial = 0;
> +
> +		block_fn(sctx, sctx->buffer, 1);
> +	}
> +
> +	memset(sctx->buffer + partial, 0x0, bit_offset - partial);
> +	*bits = cpu_to_be64(sctx->count << 3);
> +	block_fn(sctx, sctx->buffer, 1);
> +
> +	return 0;
> +}
> +
> +static inline int sha1_base_finish(struct sha1_state *sctx, u8 *out)
> +{
> +	__be32 *digest = (__be32 *)out;
> +	int i;
> +
> +	for (i = 0; i < SHA1_DIGEST_SIZE / (int)sizeof(__be32); i++)
> +		put_unaligned_be32(sctx->state[i], digest++);
> +
> +	memzero_explicit(sctx, sizeof(*sctx));
> +	return 0;
> +}
> +
> +#endif /* _CRYPTO_SHA1_BASE_H */
> diff --git a/tools/perf/util/sha1_generic.c b/tools/perf/util/sha1_generic.c
> new file mode 100644
> index 000000000000..b0a7af370d59
> --- /dev/null
> +++ b/tools/perf/util/sha1_generic.c
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Cryptographic API.
> + *
> + * SHA1 Secure Hash Algorithm.
> + *
> + * Derived from cryptoapi implementation, adapted for in-place
> + * scatterlist interface.
> + *
> + * Copyright (c) Alan Smithee.
> + * Copyright (c) Andrew McDonald <andrew@mcdonald.org.uk>
> + * Copyright (c) Jean-Francois Dive <jef@linuxbe.org>
> + */
> +#include <linux/types.h>
> +#include <linux/string.h>
> +#include <asm/byteorder.h>
> +
> +#include "sha1_base.h"
> +
> +static void sha1_generic_block_fn(struct sha1_state *sst, u8 const *src,
> +				  int blocks)
> +{
> +	u32 temp[SHA1_WORKSPACE_WORDS];
> +
> +	while (blocks--) {
> +		sha1_transform(sst->state, (const char *)src, temp);
> +		src += SHA1_BLOCK_SIZE;
> +	}
> +	memzero_explicit(temp, sizeof(temp));
> +}
> +
> +int crypto_sha1_update(struct sha1_state *desc, const u8 *data,
> +		       unsigned int len)
> +{
> +	return sha1_base_do_update(desc, data, len, sha1_generic_block_fn);
> +}
> +
> +static int sha1_final(struct sha1_state *desc, u8 *out)
> +{
> +	sha1_base_do_finalize(desc, sha1_generic_block_fn);
> +	return sha1_base_finish(desc, out);
> +}
> +
> +int crypto_sha1_finup(struct sha1_state *desc, const u8 *data,
> +		      unsigned int len, u8 *out)
> +{
> +	sha1_base_do_update(desc, data, len, sha1_generic_block_fn);
> +	return sha1_final(desc, out);
> +}
> -- 
> 2.49.0.1164.gab81da1b16-goog

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 3/4] perf genelf: Remove libcrypto dependency and use sha1 utils
  2025-05-21 22:53 ` [PATCH v1 3/4] perf genelf: Remove libcrypto dependency and use " Yuzhuo Jing
@ 2025-05-22 17:05   ` Arnaldo Carvalho de Melo
  2025-05-22 17:23     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-22 17:05 UTC (permalink / raw)
  To: Yuzhuo Jing
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	Liang Kan, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
	Justin Stitt, Steven Rostedt (Google), James Clark, Tomas Glozar,
	Leo Yan, Guilherme Amadio, Yang Jihong, Masami Hiramatsu (Google),
	Adhemerval Zanella, Wei Yang, Ard Biesheuvel,
	Mike Rapoport (Microsoft), Athira Rajeev, Kajol Jain,
	Aditya Gupta, Charlie Jenkins, Steinar H. Gunderson,
	Dr. David Alan Gilbert, Herbert Xu, Jeff Johnson, Al Viro,
	linux-kernel, linux-perf-users, llvm

On Wed, May 21, 2025 at 03:53:06PM -0700, Yuzhuo Jing wrote:
> genelf is the only file in perf that depends on libcrypto (or openssl)
> which only calculates a Build ID (SHA1, MD5, or URANDOM).  SHA1 was
> expected to be the default option, but MD5 was used by default due to
> previous issues when linking against Java.  This commit switches genelf
> to use in-house SHA1 utils, and also removes MD5 and URANDOM options
> since we have a reliable SHA1 implementation to rely on.  It passes the
> tools/perf/tests/shell/test_java_symbol.sh test.

Cool, I was going to ask if there was some 'perf test' this could be
tested with, there is, good.

- Arnaldo
 
> Signed-off-by: Yuzhuo Jing <yuzhuo@google.com>
> ---
>  tools/perf/util/genelf.c | 72 ++++------------------------------------
>  1 file changed, 6 insertions(+), 66 deletions(-)
> 
> diff --git a/tools/perf/util/genelf.c b/tools/perf/util/genelf.c
> index cdce7f173d00..cfedb29260ef 100644
> --- a/tools/perf/util/genelf.c
> +++ b/tools/perf/util/genelf.c
> @@ -28,24 +28,7 @@
>  #define NT_GNU_BUILD_ID 3
>  #endif
>  
> -#define BUILD_ID_URANDOM /* different uuid for each run */
> -
> -#ifdef HAVE_LIBCRYPTO_SUPPORT
> -
> -#define BUILD_ID_MD5
> -#undef BUILD_ID_SHA	/* does not seem to work well when linked with Java */
> -#undef BUILD_ID_URANDOM /* different uuid for each run */
> -
> -#ifdef BUILD_ID_SHA
> -#include <openssl/sha.h>
> -#endif
> -
> -#ifdef BUILD_ID_MD5
> -#include <openssl/evp.h>
> -#include <openssl/md5.h>
> -#endif
> -#endif
> -
> +#include "sha1_base.h"
>  
>  typedef struct {
>    unsigned int namesz;  /* Size of entry's owner string */
> @@ -92,64 +75,21 @@ static Elf_Sym symtab[]={
>  	}
>  };
>  
> -#ifdef BUILD_ID_URANDOM
> -static void
> -gen_build_id(struct buildid_note *note,
> -	     unsigned long load_addr __maybe_unused,
> -	     const void *code __maybe_unused,
> -	     size_t csize __maybe_unused)
> -{
> -	int fd;
> -	size_t sz = sizeof(note->build_id);
> -	ssize_t sret;
> -
> -	fd = open("/dev/urandom", O_RDONLY);
> -	if (fd == -1)
> -		err(1, "cannot access /dev/urandom for buildid");
> -
> -	sret = read(fd, note->build_id, sz);
> -
> -	close(fd);
> -
> -	if (sret != (ssize_t)sz)
> -		memset(note->build_id, 0, sz);
> -}
> -#endif
> -
> -#ifdef BUILD_ID_SHA
>  static void
>  gen_build_id(struct buildid_note *note,
>  	     unsigned long load_addr __maybe_unused,
>  	     const void *code,
>  	     size_t csize)
>  {
> -	if (sizeof(note->build_id) < SHA_DIGEST_LENGTH)
> -		errx(1, "build_id too small for SHA1");
> -
> -	SHA1(code, csize, (unsigned char *)note->build_id);
> -}
> -#endif
> -
> -#ifdef BUILD_ID_MD5
> -static void
> -gen_build_id(struct buildid_note *note, unsigned long load_addr, const void *code, size_t csize)
> -{
> -	EVP_MD_CTX *mdctx;
> +	struct sha1_state sctx;
>  
> -	if (sizeof(note->build_id) < 16)
> -		errx(1, "build_id too small for MD5");
> +	if (sizeof(note->build_id) < SHA1_DIGEST_SIZE)
> +		errx(1, "build_id too small for SHA1");
>  
> -	mdctx = EVP_MD_CTX_new();
> -	if (!mdctx)
> -		errx(2, "failed to create EVP_MD_CTX");
> +	sha1_base_init(&sctx);
>  
> -	EVP_DigestInit_ex(mdctx, EVP_md5(), NULL);
> -	EVP_DigestUpdate(mdctx, &load_addr, sizeof(load_addr));
> -	EVP_DigestUpdate(mdctx, code, csize);
> -	EVP_DigestFinal_ex(mdctx, (unsigned char *)note->build_id, NULL);
> -	EVP_MD_CTX_free(mdctx);
> +	crypto_sha1_finup(&sctx, code, csize, (unsigned char *)note->build_id);
>  }
> -#endif
>  
>  static int
>  jit_add_eh_frame_info(Elf *e, void* unwinding, uint64_t unwinding_header_size,
> -- 
> 2.49.0.1164.gab81da1b16-goog

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 3/4] perf genelf: Remove libcrypto dependency and use sha1 utils
  2025-05-22 17:05   ` Arnaldo Carvalho de Melo
@ 2025-05-22 17:23     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-22 17:23 UTC (permalink / raw)
  To: Yuzhuo Jing
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	Liang Kan, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
	Justin Stitt, Steven Rostedt (Google), James Clark, Tomas Glozar,
	Leo Yan, Guilherme Amadio, Yang Jihong, Masami Hiramatsu (Google),
	Adhemerval Zanella, Wei Yang, Ard Biesheuvel,
	Mike Rapoport (Microsoft), Athira Rajeev, Kajol Jain,
	Aditya Gupta, Charlie Jenkins, Steinar H. Gunderson,
	Dr. David Alan Gilbert, Herbert Xu, Jeff Johnson, Al Viro,
	linux-kernel, linux-perf-users, llvm

On Thu, May 22, 2025 at 02:05:17PM -0300, Arnaldo Carvalho de Melo wrote:
> On Wed, May 21, 2025 at 03:53:06PM -0700, Yuzhuo Jing wrote:
> > genelf is the only file in perf that depends on libcrypto (or openssl)
> > which only calculates a Build ID (SHA1, MD5, or URANDOM).  SHA1 was
> > expected to be the default option, but MD5 was used by default due to
> > previous issues when linking against Java.  This commit switches genelf
> > to use in-house SHA1 utils, and also removes MD5 and URANDOM options
> > since we have a reliable SHA1 implementation to rely on.  It passes the
> > tools/perf/tests/shell/test_java_symbol.sh test.

> Cool, I was going to ask if there was some 'perf test' this could be
> tested with, there is, good.

I applied the series locally and while testing on a new machine I
noticed some issues with the existing test_java_symbol.sh test, namely:

root@number:~# perf test -vvvv "Test java symbol"
105: Test java symbol:
--- start ---
test child forked, pid 85204
skip: no jshell, install JDK
---- end(-2) ----
105: Test java symbol                                                : Skip
root@number:~# 

So I had to re-figure out where that thing was, after I installed
java-latest-openjdk-devel the test passed, so I'll write a follow up
patch that will add a the reason for the skip suggesting that the needed
devel file be installed.

Also using -vvv:

root@number:~# perf test "Test java symbol"
105: Test java symbol                                                : Ok
root@number:~# perf test -vvvvv "Test java symbol"
105: Test java symbol:
--- start ---
test child forked, pid 86083
jshell: jvmti: jitdump in /root/.debug/jit/java-jit-20250522.XXYVwxbq/jit-86090.dump
-> int fib(int x) {
>>      return x > 1 ? fib(x - 2) + fib(x - 1) : 1;
>>     -> -> -> -> -> -> 143
-> [ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.436 MB /tmp/__perf_test.perf.data.zt9Jv (10113 samples) ]
Cleaning up files...
---- end(0) ----
105: Test java symbol                                                : Ok
root@number:~# 
root@number:~# ls -la /root/.debug/jit/java-jit-20250522.XXYVwxbq/jit-86090.dump
-rw-r--r--. 1 root root 13065557 May 22 14:18 /root/.debug/jit/java-jit-20250522.XXYVwxbq/jit-86090.dump
root@number:~#

It creates that jit-86090.dump file but leaves it in the ~/.debug/
directory, polluting it, it should remove it at the test exit.

None are issues introduced by your patch set, so should be fixed as
followup patches that I'll do after processing more outstanding patches
unless someone beats me to it ;-)

I'm keeping your patches in my local repo to give some more time for
reviewing, they will go thru container tests in lots of distros
meanwhile.

Thanks,

- Arnaldo
 
> - Arnaldo
>  
> > Signed-off-by: Yuzhuo Jing <yuzhuo@google.com>
> > ---
> >  tools/perf/util/genelf.c | 72 ++++------------------------------------
> >  1 file changed, 6 insertions(+), 66 deletions(-)
> > 
> > diff --git a/tools/perf/util/genelf.c b/tools/perf/util/genelf.c
> > index cdce7f173d00..cfedb29260ef 100644
> > --- a/tools/perf/util/genelf.c
> > +++ b/tools/perf/util/genelf.c
> > @@ -28,24 +28,7 @@
> >  #define NT_GNU_BUILD_ID 3
> >  #endif
> >  
> > -#define BUILD_ID_URANDOM /* different uuid for each run */
> > -
> > -#ifdef HAVE_LIBCRYPTO_SUPPORT
> > -
> > -#define BUILD_ID_MD5
> > -#undef BUILD_ID_SHA	/* does not seem to work well when linked with Java */
> > -#undef BUILD_ID_URANDOM /* different uuid for each run */
> > -
> > -#ifdef BUILD_ID_SHA
> > -#include <openssl/sha.h>
> > -#endif
> > -
> > -#ifdef BUILD_ID_MD5
> > -#include <openssl/evp.h>
> > -#include <openssl/md5.h>
> > -#endif
> > -#endif
> > -
> > +#include "sha1_base.h"
> >  
> >  typedef struct {
> >    unsigned int namesz;  /* Size of entry's owner string */
> > @@ -92,64 +75,21 @@ static Elf_Sym symtab[]={
> >  	}
> >  };
> >  
> > -#ifdef BUILD_ID_URANDOM
> > -static void
> > -gen_build_id(struct buildid_note *note,
> > -	     unsigned long load_addr __maybe_unused,
> > -	     const void *code __maybe_unused,
> > -	     size_t csize __maybe_unused)
> > -{
> > -	int fd;
> > -	size_t sz = sizeof(note->build_id);
> > -	ssize_t sret;
> > -
> > -	fd = open("/dev/urandom", O_RDONLY);
> > -	if (fd == -1)
> > -		err(1, "cannot access /dev/urandom for buildid");
> > -
> > -	sret = read(fd, note->build_id, sz);
> > -
> > -	close(fd);
> > -
> > -	if (sret != (ssize_t)sz)
> > -		memset(note->build_id, 0, sz);
> > -}
> > -#endif
> > -
> > -#ifdef BUILD_ID_SHA
> >  static void
> >  gen_build_id(struct buildid_note *note,
> >  	     unsigned long load_addr __maybe_unused,
> >  	     const void *code,
> >  	     size_t csize)
> >  {
> > -	if (sizeof(note->build_id) < SHA_DIGEST_LENGTH)
> > -		errx(1, "build_id too small for SHA1");
> > -
> > -	SHA1(code, csize, (unsigned char *)note->build_id);
> > -}
> > -#endif
> > -
> > -#ifdef BUILD_ID_MD5
> > -static void
> > -gen_build_id(struct buildid_note *note, unsigned long load_addr, const void *code, size_t csize)
> > -{
> > -	EVP_MD_CTX *mdctx;
> > +	struct sha1_state sctx;
> >  
> > -	if (sizeof(note->build_id) < 16)
> > -		errx(1, "build_id too small for MD5");
> > +	if (sizeof(note->build_id) < SHA1_DIGEST_SIZE)
> > +		errx(1, "build_id too small for SHA1");
> >  
> > -	mdctx = EVP_MD_CTX_new();
> > -	if (!mdctx)
> > -		errx(2, "failed to create EVP_MD_CTX");
> > +	sha1_base_init(&sctx);
> >  
> > -	EVP_DigestInit_ex(mdctx, EVP_md5(), NULL);
> > -	EVP_DigestUpdate(mdctx, &load_addr, sizeof(load_addr));
> > -	EVP_DigestUpdate(mdctx, code, csize);
> > -	EVP_DigestFinal_ex(mdctx, (unsigned char *)note->build_id, NULL);
> > -	EVP_MD_CTX_free(mdctx);
> > +	crypto_sha1_finup(&sctx, code, csize, (unsigned char *)note->build_id);
> >  }
> > -#endif
> >  
> >  static int
> >  jit_add_eh_frame_info(Elf *e, void* unwinding, uint64_t unwinding_header_size,
> > -- 
> > 2.49.0.1164.gab81da1b16-goog

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 4/4] tools: Remove libcrypto dependency
  2025-05-21 22:53 ` [PATCH v1 4/4] tools: Remove libcrypto dependency Yuzhuo Jing
@ 2025-05-22 17:30   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-22 17:30 UTC (permalink / raw)
  To: Yuzhuo Jing
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	Liang Kan, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
	Justin Stitt, Steven Rostedt (Google), James Clark, Tomas Glozar,
	Leo Yan, Guilherme Amadio, Yang Jihong, Masami Hiramatsu (Google),
	Adhemerval Zanella, Wei Yang, Ard Biesheuvel,
	Mike Rapoport (Microsoft), Athira Rajeev, Kajol Jain,
	Aditya Gupta, Charlie Jenkins, Steinar H. Gunderson,
	Dr. David Alan Gilbert, Herbert Xu, Jeff Johnson, Al Viro,
	linux-kernel, linux-perf-users, llvm

On Wed, May 21, 2025 at 03:53:07PM -0700, Yuzhuo Jing wrote:
> Remove all occurrence of libcrypto in the build system.

and that was so far used only by tools/perf, good:

⬢ [acme@toolbx perf-tools-next]$ git grep -w feature-libcrypto tools/

As there are some other features that are tested/used by other tools/
living code.

⬢ [acme@toolbx perf-tools-next]$ git grep -w feature-libbfd tools/
tools/bpf/bpftool/Makefile:  ifeq ($(feature-libbfd),1)
tools/bpf/bpftool/Makefile:  else ifeq ($(feature-libbfd-liberty),1)
tools/bpf/bpftool/Makefile:  else ifeq ($(feature-libbfd-liberty-z),1)
tools/perf/Makefile.config:  ifeq ($(feature-libbfd), 1)
tools/perf/Makefile.config:    ifeq ($(feature-libbfd-liberty), 1)
tools/perf/Makefile.config:      ifeq ($(feature-libbfd-liberty-z), 1)
tools/perf/Makefile.config:  ifeq ($(feature-libbfd-buildid), 1)
⬢ [acme@toolbx perf-tools-next]$

- Arnaldo
 
> Signed-off-by: Yuzhuo Jing <yuzhuo@google.com>
> ---
>  tools/build/Makefile.feature            |  2 --
>  tools/build/feature/Makefile            |  4 ----
>  tools/build/feature/test-all.c          |  5 -----
>  tools/build/feature/test-libcrypto.c    | 25 -------------------------
>  tools/perf/Documentation/perf-check.txt |  1 -
>  tools/perf/Makefile.config              | 13 -------------
>  tools/perf/Makefile.perf                |  3 ---
>  tools/perf/builtin-check.c              |  1 -
>  tools/perf/tests/make                   |  4 +---
>  9 files changed, 1 insertion(+), 57 deletions(-)
>  delete mode 100644 tools/build/feature/test-libcrypto.c
> 
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 57bd995ce6af..bbadfb5568eb 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -86,7 +86,6 @@ FEATURE_TESTS_BASIC :=                  \
>          libtraceevent                   \
>          libtracefs                      \
>          libcpupower                     \
> -        libcrypto                       \
>          pthread-attr-setaffinity-np     \
>          pthread-barrier     		\
>          reallocarray                    \
> @@ -152,7 +151,6 @@ FEATURE_DISPLAY ?=              \
>           numa_num_possible_cpus \
>           libperl                \
>           libpython              \
> -         libcrypto              \
>           libcapstone            \
>           llvm-perf              \
>           zlib                   \
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index b8b5fb183dd4..04a4aa0341aa 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -38,7 +38,6 @@ FILES=                                          \
>           test-libtraceevent.bin                 \
>           test-libcpupower.bin                   \
>           test-libtracefs.bin                    \
> -         test-libcrypto.bin                     \
>           test-libunwind.bin                     \
>           test-libunwind-debug-frame.bin         \
>           test-libunwind-x86.bin                 \
> @@ -246,9 +245,6 @@ $(OUTPUT)test-libcpupower.bin:
>  $(OUTPUT)test-libtracefs.bin:
>  	 $(BUILD) $(shell $(PKG_CONFIG) --cflags libtracefs 2>/dev/null) -ltracefs
>  
> -$(OUTPUT)test-libcrypto.bin:
> -	$(BUILD) -lcrypto
> -
>  $(OUTPUT)test-gtk2.bin:
>  	$(BUILD) $(shell $(PKG_CONFIG) --libs --cflags gtk+-2.0 2>/dev/null) -Wno-deprecated-declarations
>  
> diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
> index 03ddaac6f4c4..ce72e2061ac0 100644
> --- a/tools/build/feature/test-all.c
> +++ b/tools/build/feature/test-all.c
> @@ -138,10 +138,6 @@
>  # include "test-bpf.c"
>  #undef main
>  
> -#define main main_test_libcrypto
> -# include "test-libcrypto.c"
> -#undef main
> -
>  #define main main_test_sdt
>  # include "test-sdt.c"
>  #undef main
> @@ -206,7 +202,6 @@ int main(int argc, char *argv[])
>  	main_test_lzma();
>  	main_test_get_cpuid();
>  	main_test_bpf();
> -	main_test_libcrypto();
>  	main_test_scandirat();
>  	main_test_sched_getcpu();
>  	main_test_sdt();
> diff --git a/tools/build/feature/test-libcrypto.c b/tools/build/feature/test-libcrypto.c
> deleted file mode 100644
> index bc34a5bbb504..000000000000
> --- a/tools/build/feature/test-libcrypto.c
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -#include <openssl/evp.h>
> -#include <openssl/sha.h>
> -#include <openssl/md5.h>
> -
> -int main(void)
> -{
> -	EVP_MD_CTX *mdctx;
> -	unsigned char md[MD5_DIGEST_LENGTH + SHA_DIGEST_LENGTH];
> -	unsigned char dat[] = "12345";
> -	unsigned int digest_len;
> -
> -	mdctx = EVP_MD_CTX_new();
> -	if (!mdctx)
> -		return 0;
> -
> -	EVP_DigestInit_ex(mdctx, EVP_md5(), NULL);
> -	EVP_DigestUpdate(mdctx, &dat[0], sizeof(dat));
> -	EVP_DigestFinal_ex(mdctx, &md[0], &digest_len);
> -	EVP_MD_CTX_free(mdctx);
> -
> -	SHA1(&dat[0], sizeof(dat), &md[0]);
> -
> -	return 0;
> -}
> diff --git a/tools/perf/Documentation/perf-check.txt b/tools/perf/Documentation/perf-check.txt
> index a764a4629220..2b96cb578658 100644
> --- a/tools/perf/Documentation/perf-check.txt
> +++ b/tools/perf/Documentation/perf-check.txt
> @@ -53,7 +53,6 @@ feature::
>                  auxtrace                /  HAVE_AUXTRACE_SUPPORT
>                  libbfd                  /  HAVE_LIBBFD_SUPPORT
>                  libcapstone             /  HAVE_LIBCAPSTONE_SUPPORT
> -                libcrypto               /  HAVE_LIBCRYPTO_SUPPORT
>                  libdw-dwarf-unwind      /  HAVE_LIBDW_SUPPORT
>                  libelf                  /  HAVE_LIBELF_SUPPORT
>                  libnuma                 /  HAVE_LIBNUMA_SUPPORT
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index d1ea7bf44964..d19d1f132726 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -130,8 +130,6 @@ ifndef NO_LIBUNWIND
>    FEATURE_CHECK_LDFLAGS-libunwind-x86_64 += -lunwind -llzma -lunwind-x86_64
>  endif
>  
> -FEATURE_CHECK_LDFLAGS-libcrypto = -lcrypto
> -
>  ifdef CSINCLUDES
>    LIBOPENCSD_CFLAGS := -I$(CSINCLUDES)
>  endif
> @@ -772,17 +770,6 @@ ifneq ($(NO_LIBTRACEEVENT),1)
>    $(call detected,CONFIG_TRACE)
>  endif
>  
> -ifndef NO_LIBCRYPTO
> -  ifneq ($(feature-libcrypto), 1)
> -    $(warning No libcrypto.h found, disables jitted code injection, please install openssl-devel or libssl-dev)
> -    NO_LIBCRYPTO := 1
> -  else
> -    CFLAGS += -DHAVE_LIBCRYPTO_SUPPORT
> -    EXTLIBS += -lcrypto
> -    $(call detected,CONFIG_CRYPTO)
> -  endif
> -endif
> -
>  ifndef NO_SLANG
>    ifneq ($(feature-libslang), 1)
>      ifneq ($(feature-libslang-include-subdir), 1)
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index d4c7031b01a7..9246c94656e0 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -61,9 +61,6 @@ include ../scripts/utilities.mak
>  #
>  # Define NO_LIBBIONIC if you do not want bionic support
>  #
> -# Define NO_LIBCRYPTO if you do not want libcrypto (openssl) support
> -# used for generating build-ids for ELFs generated by jitdump.
> -#
>  # Define NO_LIBDW_DWARF_UNWIND if you do not want libdw support
>  # for dwarf backtrace post unwind.
>  #
> diff --git a/tools/perf/builtin-check.c b/tools/perf/builtin-check.c
> index 9a509cb3bb9a..ad49f2564aae 100644
> --- a/tools/perf/builtin-check.c
> +++ b/tools/perf/builtin-check.c
> @@ -44,7 +44,6 @@ struct feature_status supported_features[] = {
>  	FEATURE_STATUS("auxtrace", HAVE_AUXTRACE_SUPPORT),
>  	FEATURE_STATUS_TIP("libbfd", HAVE_LIBBFD_SUPPORT, "Deprecated, license incompatibility, use BUILD_NONDISTRO=1 and install binutils-dev[el]"),
>  	FEATURE_STATUS("libcapstone", HAVE_LIBCAPSTONE_SUPPORT),
> -	FEATURE_STATUS("libcrypto", HAVE_LIBCRYPTO_SUPPORT),
>  	FEATURE_STATUS("libdw-dwarf-unwind", HAVE_LIBDW_SUPPORT),
>  	FEATURE_STATUS("libelf", HAVE_LIBELF_SUPPORT),
>  	FEATURE_STATUS("libnuma", HAVE_LIBNUMA_SUPPORT),
> diff --git a/tools/perf/tests/make b/tools/perf/tests/make
> index 0ee94caf9ec1..e3651e5b195a 100644
> --- a/tools/perf/tests/make
> +++ b/tools/perf/tests/make
> @@ -91,7 +91,6 @@ make_no_auxtrace    := NO_AUXTRACE=1
>  make_no_libbpf	    := NO_LIBBPF=1
>  make_libbpf_dynamic := LIBBPF_DYNAMIC=1
>  make_no_libbpf_DEBUG := NO_LIBBPF=1 DEBUG=1
> -make_no_libcrypto   := NO_LIBCRYPTO=1
>  make_no_libllvm     := NO_LIBLLVM=1
>  make_with_babeltrace:= LIBBABELTRACE=1
>  make_with_coresight := CORESIGHT=1
> @@ -122,7 +121,7 @@ make_minimal        := NO_LIBPERL=1 NO_LIBPYTHON=1 NO_GTK2=1
>  make_minimal        += NO_DEMANGLE=1 NO_LIBELF=1 NO_BACKTRACE=1
>  make_minimal        += NO_LIBNUMA=1 NO_LIBBIONIC=1
>  make_minimal        += NO_LIBDW_DWARF_UNWIND=1 NO_AUXTRACE=1 NO_LIBBPF=1
> -make_minimal        += NO_LIBCRYPTO=1 NO_SDT=1 NO_JVMTI=1 NO_LIBZSTD=1
> +make_minimal        += NO_SDT=1 NO_JVMTI=1 NO_LIBZSTD=1
>  make_minimal        += NO_LIBCAP=1 NO_CAPSTONE=1
>  
>  # $(run) contains all available tests
> @@ -160,7 +159,6 @@ run += make_no_libbionic
>  run += make_no_auxtrace
>  run += make_no_libbpf
>  run += make_no_libbpf_DEBUG
> -run += make_no_libcrypto
>  run += make_no_libllvm
>  run += make_no_sdt
>  run += make_no_syscall_tbl
> -- 
> 2.49.0.1164.gab81da1b16-goog

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 2/4] perf tools: Add sha1 utils
  2025-05-21 22:53 ` [PATCH v1 2/4] perf tools: Add " Yuzhuo Jing
  2025-05-22 17:03   ` Arnaldo Carvalho de Melo
@ 2025-05-22 17:56   ` Arnaldo Carvalho de Melo
  2025-06-04 18:17     ` Yuzhuo Jing
  1 sibling, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-22 17:56 UTC (permalink / raw)
  To: Yuzhuo Jing
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	Liang Kan, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
	Justin Stitt, Steven Rostedt (Google), James Clark, Tomas Glozar,
	Leo Yan, Guilherme Amadio, Yang Jihong, Masami Hiramatsu (Google),
	Adhemerval Zanella, Wei Yang, Ard Biesheuvel,
	Mike Rapoport (Microsoft), Athira Rajeev, Kajol Jain,
	Aditya Gupta, Charlie Jenkins, Steinar H. Gunderson,
	Dr. David Alan Gilbert, Herbert Xu, Jeff Johnson, Al Viro,
	linux-kernel, linux-perf-users, llvm

On Wed, May 21, 2025 at 03:53:05PM -0700, Yuzhuo Jing wrote:
> Those new files are derived from the kernel tree, namely:
> 
>   tools/perf/util/sha1.c          from  lib/crypto/sha1.c
>   tools/perf/util/sha1.h          from  include/crypto/sha1.h
>   tools/perf/util/sha1_base.h     from  include/crypto/sha1_base.h
>   tools/perf/util/sha1_generic.c  from  crypto/sha1_generic.c
> 
> The reason that we are not syncing them with the kernel tree like other
> tools header files is because of the deep dependency in
> include/crypto/hash.h.  It's painful to import the whole kernel crypto
> driver infrastructure into tools.

This is failing:

⬢ [acme@toolbx perf-tools-next]$ git log --oneline -1 ; time make -C tools/perf build-test
<SNIP>
           make_no_scripts_O: cd . && make NO_LIBPYTHON=1 NO_LIBPERL=1 FEATURES_DUMP=/home/acme/git/perf-tools-next/tools/perf/BUILD_TEST_FEATURE_DUMP -j32 O=/tmp/tmp.aknNhb7DZp DESTDIR=/tmp/tmp.8qecgbhwKa
          make_extra_tests_O: cd . && make EXTRA_TESTS=1 FEATURES_DUMP=/home/acme/git/perf-tools-next/tools/perf/BUILD_TEST_FEATURE_DUMP -j32 O=/tmp/tmp.C1nM6OCHSS DESTDIR=/tmp/tmp.iWigrG5sDJ
                make_debug_O: cd . && make DEBUG=1 FEATURES_DUMP=/home/acme/git/perf-tools-next/tools/perf/BUILD_TEST_FEATURE_DUMP -j32 O=/tmp/tmp.z8p7Unts6E DESTDIR=/tmp/tmp.k9856UvMRF
cd . && make DEBUG=1 FEATURES_DUMP=/home/acme/git/perf-tools-next/tools/perf/BUILD_TEST_FEATURE_DUMP -j32 O=/tmp/tmp.z8p7Unts6E DESTDIR=/tmp/tmp.k9856UvMRF
  BUILD:   Doing 'make -j32' parallel build
Warning: Kernel ABI header differences:
  diff -u tools/arch/arm64/include/asm/cputype.h arch/arm64/include/asm/cputype.h
Makefile.config:955: No libllvm 13+ found, slower source file resolution, please install llvm-devel/llvm-dev
Makefile.config:1134: No openjdk development package found, please install JDK package, e.g. openjdk-8-jdk, java-1.8.0-openjdk-devel

  GEN     /tmp/tmp.z8p7Unts6E/common-cmds.h
  CC      /tmp/tmp.z8p7Unts6E/dlfilters/dlfilter-test-api-v0.o
  CC      /tmp/tmp.z8p7Unts6E/dlfilters/dlfilter-test-api-v2.o
<SNIP>
  CC      /tmp/tmp.z8p7Unts6E/util/strlist.o
  CC      /tmp/tmp.z8p7Unts6E/tests/demangle-java-test.o
  LD      /tmp/tmp.z8p7Unts6E/trace/beauty/tracepoints/perf-in.o
In file included from util/sha1_generic.c:18:
util/sha1_base.h: In function ‘sha1_base_do_finalize’:
util/sha1_base.h:77:21: error: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
   77 |         if (partial > bit_offset) {
      |                     ^
cc1: all warnings being treated as errors
  CC      /tmp/tmp.z8p7Unts6E/tests/demangle-ocaml-test.o
  CC      /tmp/tmp.z8p7Unts6E/util/strfilter.o
make[6]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:86: /tmp/tmp.z8p7Unts6E/util/sha1_generic.o] Error 1
make[6]: *** Waiting for unfinished jobs....
  LD      /tmp/tmp.z8p7Unts6E/trace/beauty/perf-in.o
  CC      /tmp/tmp.z8p7Unts6E/tests/demangle-rust-v0-test.o
<SNIP>
  LD      /tmp/tmp.z8p7Unts6E/util/scripting-engines/perf-util-in.o
make[5]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:142: util] Error 2
make[4]: *** [Makefile.perf:795: /tmp/tmp.z8p7Unts6E/perf-util-in.o] Error 2
make[4]: *** Waiting for unfinished jobs....
  CC      /tmp/tmp.z8p7Unts6E/pmu-events/pmu-events.o
  LD      /tmp/tmp.z8p7Unts6E/pmu-events/pmu-events-in.o
make[3]: *** [Makefile.perf:287: sub-make] Error 2
make[2]: *** [Makefile:76: all] Error 2
make[1]: *** [tests/make:339: make_debug_O] Error 1
make: *** [Makefile:109: build-test] Error 2
make: Leaving directory '/home/acme/git/perf-tools-next/tools/perf'

real	2m10.250s
user	20m47.352s
sys	3m50.484s
⬢ [acme@toolbx perf-tools-next]$

Humm that part is the same as in the kernel...

⬢ [acme@toolbx perf-tools-next]$ line=$(ctags -x --c-kinds=f include/crypto/sha1_base.h | awk '$1 == "sha1_base_do_finalize" {print $3}')
⬢ [acme@toolbx perf-tools-next]$ sed -n $line,\$p include/crypto/sha1_base.h | awk '{print} /\{/ {c++} /\}/ {c--; if (c==0) exit}' > /tmp/original
⬢ [acme@toolbx perf-tools-next]$ line=$(ctags -x --c-kinds=f tools/perf/util/sha1_base.h | awk '$1 == "sha1_base_do_finalize" {print $3}')
⬢ [acme@toolbx perf-tools-next]$ sed -n $line,\$p include/crypto/sha1_base.h | awk '{print} /\{/ {c++} /\}/ {c--; if (c==0) exit}' > /tmp/copy
⬢ [acme@toolbx perf-tools-next]$ diff -u /tmp/original /tmp/copy
--- /tmp/original	2025-05-22 14:48:31.338406860 -0300
+++ /tmp/copy	2025-05-22 14:48:58.401603439 -0300
@@ -1,3 +1,7 @@
+
+	return 0;
+}
+
 static inline int sha1_base_do_finalize(struct shash_desc *desc,
 					sha1_block_fn *block_fn)
 {
@@ -13,10 +17,3 @@
 
 		block_fn(sctx, sctx->buffer, 1);
 	}
-
-	memset(sctx->buffer + partial, 0x0, bit_offset - partial);
-	*bits = cpu_to_be64(sctx->count << 3);
-	block_fn(sctx, sctx->buffer, 1);
-
-	return 0;
-}
⬢ [acme@toolbx perf-tools-next]$ 

If I try the quick fix:

⬢ [acme@toolbx perf-tools-next]$ git diff
diff --git a/tools/perf/util/sha1_base.h b/tools/perf/util/sha1_base.h
index cea22c5a49520998..5994c55748e40a79 100644
--- a/tools/perf/util/sha1_base.h
+++ b/tools/perf/util/sha1_base.h
@@ -71,7 +71,7 @@ static inline int sha1_base_do_finalize(struct sha1_state *sctx,
 {
        const int bit_offset = SHA1_BLOCK_SIZE - sizeof(__be64);
        __be64 *bits = (__be64 *)(sctx->buffer + bit_offset);
-       unsigned int partial = sctx->count % SHA1_BLOCK_SIZE;
+       int partial = sctx->count % SHA1_BLOCK_SIZE;
 
        sctx->buffer[partial++] = 0x80;
        if (partial > bit_offset) {
⬢ [acme@toolbx perf-tools-next]$

IT works, to test it quicker try, before the above:

$ make -C tools/perf -f tests/make make_debug
<SNIP>
 CC      util/bpf_map.o
In file included from util/sha1_generic.c:18:
util/sha1_base.h: In function ‘sha1_base_do_finalize’:
util/sha1_base.h:77:21: error: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
   77 |         if (partial > bit_offset) {
      |                     ^
cc1: all warnings being treated as errors
<SNIP>

After the above patch:

⬢ [acme@toolbx perf-tools-next]$ rm -f tools/perf/make_debug tools/perf/make_debug_O
⬢ [acme@toolbx perf-tools-next]$ make -C tools/perf -f tests/make make_debug
make: Entering directory '/home/acme/git/perf-tools-next/tools/perf'
Testing Makefile
                  make_debug: cd . && make DEBUG=1   DESTDIR=/tmp/tmp.yfg6I4SlFY
make: Leaving directory '/home/acme/git/perf-tools-next/tools/perf'
⬢ [acme@toolbx perf-tools-next]$ 

- Arnaldo

 
> The derived files get rid of struct shash_desc definition, and directly
> operates on the struct sha1_state.
> 
> Signed-off-by: Yuzhuo Jing <yuzhuo@google.com>
> ---
>  tools/perf/util/Build          |   2 +
>  tools/perf/util/sha1.c         | 122 +++++++++++++++++++++++++++++++++
>  tools/perf/util/sha1.h         |  41 +++++++++++
>  tools/perf/util/sha1_base.h    | 103 ++++++++++++++++++++++++++++
>  tools/perf/util/sha1_generic.c |  49 +++++++++++++
>  5 files changed, 317 insertions(+)
>  create mode 100644 tools/perf/util/sha1.c
>  create mode 100644 tools/perf/util/sha1.h
>  create mode 100644 tools/perf/util/sha1_base.h
>  create mode 100644 tools/perf/util/sha1_generic.c
> 
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 7910d908c814..ecee96b3f3fa 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -41,6 +41,8 @@ perf-util-y += rbtree.o
>  perf-util-y += libstring.o
>  perf-util-y += bitmap.o
>  perf-util-y += hweight.o
> +perf-util-y += sha1.o
> +perf-util-y += sha1_generic.o
>  perf-util-y += smt.o
>  perf-util-y += strbuf.o
>  perf-util-y += string.o
> diff --git a/tools/perf/util/sha1.c b/tools/perf/util/sha1.c
> new file mode 100644
> index 000000000000..5ae658afb56b
> --- /dev/null
> +++ b/tools/perf/util/sha1.c
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SHA1 routine optimized to do word accesses rather than byte accesses,
> + * and to avoid unnecessary copies into the context array.
> + *
> + * This was based on the git SHA1 implementation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/bitops.h>
> +#include <linux/string.h>
> +#include <linux/unaligned.h>
> +
> +#include "sha1.h"
> +
> +/*
> + * If you have 32 registers or more, the compiler can (and should)
> + * try to change the array[] accesses into registers. However, on
> + * machines with less than ~25 registers, that won't really work,
> + * and at least gcc will make an unholy mess of it.
> + *
> + * So to avoid that mess which just slows things down, we force
> + * the stores to memory to actually happen (we might be better off
> + * with a 'W(t)=(val);asm("":"+m" (W(t))' there instead, as
> + * suggested by Artur Skawina - that will also make gcc unable to
> + * try to do the silly "optimize away loads" part because it won't
> + * see what the value will be).
> + *
> + * Ben Herrenschmidt reports that on PPC, the C version comes close
> + * to the optimized asm with this (ie on PPC you don't want that
> + * 'volatile', since there are lots of registers).
> + *
> + * On ARM we get the best code generation by forcing a full memory barrier
> + * between each SHA_ROUND, otherwise gcc happily get wild with spilling and
> + * the stack frame size simply explode and performance goes down the drain.
> + */
> +
> +#ifdef CONFIG_X86
> +  #define setW(x, val) (*(volatile __u32 *)&W(x) = (val))
> +#elif defined(CONFIG_ARM)
> +  #define setW(x, val) do { W(x) = (val); __asm__("":::"memory"); } while (0)
> +#else
> +  #define setW(x, val) (W(x) = (val))
> +#endif
> +
> +/* This "rolls" over the 512-bit array */
> +#define W(x) (array[(x)&15])
> +
> +/*
> + * Where do we get the source from? The first 16 iterations get it from
> + * the input data, the next mix it from the 512-bit array.
> + */
> +#define SHA_SRC(t) get_unaligned_be32((__u32 *)data + t)
> +#define SHA_MIX(t) rol32(W(t+13) ^ W(t+8) ^ W(t+2) ^ W(t), 1)
> +
> +#define SHA_ROUND(t, input, fn, constant, A, B, C, D, E) do { \
> +	__u32 TEMP = input(t); setW(t, TEMP); \
> +	E += TEMP + rol32(A,5) + (fn) + (constant); \
> +	B = ror32(B, 2); \
> +	TEMP = E; E = D; D = C; C = B; B = A; A = TEMP; } while (0)
> +
> +#define T_0_15(t, A, B, C, D, E)  SHA_ROUND(t, SHA_SRC, (((C^D)&B)^D) , 0x5a827999, A, B, C, D, E )
> +#define T_16_19(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (((C^D)&B)^D) , 0x5a827999, A, B, C, D, E )
> +#define T_20_39(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (B^C^D) , 0x6ed9eba1, A, B, C, D, E )
> +#define T_40_59(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, ((B&C)+(D&(B^C))) , 0x8f1bbcdc, A, B, C, D, E )
> +#define T_60_79(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (B^C^D) ,  0xca62c1d6, A, B, C, D, E )
> +
> +/**
> + * sha1_transform - single block SHA1 transform (deprecated)
> + *
> + * @digest: 160 bit digest to update
> + * @data:   512 bits of data to hash
> + * @array:  16 words of workspace (see note)
> + *
> + * This function executes SHA-1's internal compression function.  It updates the
> + * 160-bit internal state (@digest) with a single 512-bit data block (@data).
> + *
> + * Don't use this function.  SHA-1 is no longer considered secure.  And even if
> + * you do have to use SHA-1, this isn't the correct way to hash something with
> + * SHA-1 as this doesn't handle padding and finalization.
> + *
> + * Note: If the hash is security sensitive, the caller should be sure
> + * to clear the workspace. This is left to the caller to avoid
> + * unnecessary clears between chained hashing operations.
> + */
> +void sha1_transform(__u32 *digest, const char *data, __u32 *array)
> +{
> +	__u32 A, B, C, D, E;
> +	unsigned int i = 0;
> +
> +	A = digest[0];
> +	B = digest[1];
> +	C = digest[2];
> +	D = digest[3];
> +	E = digest[4];
> +
> +	/* Round 1 - iterations 0-16 take their input from 'data' */
> +	for (; i < 16; ++i)
> +		T_0_15(i, A, B, C, D, E);
> +
> +	/* Round 1 - tail. Input from 512-bit mixing array */
> +	for (; i < 20; ++i)
> +		T_16_19(i, A, B, C, D, E);
> +
> +	/* Round 2 */
> +	for (; i < 40; ++i)
> +		T_20_39(i, A, B, C, D, E);
> +
> +	/* Round 3 */
> +	for (; i < 60; ++i)
> +		T_40_59(i, A, B, C, D, E);
> +
> +	/* Round 4 */
> +	for (; i < 80; ++i)
> +		T_60_79(i, A, B, C, D, E);
> +
> +	digest[0] += A;
> +	digest[1] += B;
> +	digest[2] += C;
> +	digest[3] += D;
> +	digest[4] += E;
> +}
> diff --git a/tools/perf/util/sha1.h b/tools/perf/util/sha1.h
> new file mode 100644
> index 000000000000..9da4ece49bc6
> --- /dev/null
> +++ b/tools/perf/util/sha1.h
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Common values for SHA-1 algorithms
> + */
> +
> +#ifndef _CRYPTO_SHA1_H
> +#define _CRYPTO_SHA1_H
> +
> +#include <linux/types.h>
> +
> +#define SHA1_DIGEST_SIZE        20
> +#define SHA1_BLOCK_SIZE         64
> +
> +#define SHA1_H0		0x67452301UL
> +#define SHA1_H1		0xefcdab89UL
> +#define SHA1_H2		0x98badcfeUL
> +#define SHA1_H3		0x10325476UL
> +#define SHA1_H4		0xc3d2e1f0UL
> +
> +struct sha1_state {
> +	u32 state[SHA1_DIGEST_SIZE / 4];
> +	u64 count;
> +	u8 buffer[SHA1_BLOCK_SIZE];
> +};
> +
> +extern int crypto_sha1_update(struct sha1_state *desc, const u8 *data,
> +			      unsigned int len);
> +
> +extern int crypto_sha1_finup(struct sha1_state *desc, const u8 *data,
> +			     unsigned int len, u8 *hash);
> +
> +/*
> + * An implementation of SHA-1's compression function.  Don't use in new code!
> + * You shouldn't be using SHA-1, and even if you *have* to use SHA-1, this isn't
> + * the correct way to hash something with SHA-1 (use crypto_shash instead).
> + */
> +#define SHA1_DIGEST_WORDS	(SHA1_DIGEST_SIZE / 4)
> +#define SHA1_WORKSPACE_WORDS	16
> +void sha1_transform(__u32 *digest, const char *data, __u32 *W);
> +
> +#endif /* _CRYPTO_SHA1_H */
> diff --git a/tools/perf/util/sha1_base.h b/tools/perf/util/sha1_base.h
> new file mode 100644
> index 000000000000..cea22c5a4952
> --- /dev/null
> +++ b/tools/perf/util/sha1_base.h
> @@ -0,0 +1,103 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * sha1_base.h - core logic for SHA-1 implementations
> + *
> + * Copyright (C) 2015 Linaro Ltd <ard.biesheuvel@linaro.org>
> + */
> +
> +#ifndef _CRYPTO_SHA1_BASE_H
> +#define _CRYPTO_SHA1_BASE_H
> +
> +#include <linux/string.h>
> +
> +#include <linux/kernel.h>
> +#include <linux/unaligned.h>
> +
> +#include "sha1.h"
> +
> +typedef void (sha1_block_fn)(struct sha1_state *sst, u8 const *src, int blocks);
> +
> +static inline int sha1_base_init(struct sha1_state *sctx)
> +{
> +	sctx->state[0] = SHA1_H0;
> +	sctx->state[1] = SHA1_H1;
> +	sctx->state[2] = SHA1_H2;
> +	sctx->state[3] = SHA1_H3;
> +	sctx->state[4] = SHA1_H4;
> +	sctx->count = 0;
> +
> +	return 0;
> +}
> +
> +static inline int sha1_base_do_update(struct sha1_state *sctx,
> +				      const u8 *data,
> +				      unsigned int len,
> +				      sha1_block_fn *block_fn)
> +{
> +	unsigned int partial = sctx->count % SHA1_BLOCK_SIZE;
> +
> +	sctx->count += len;
> +
> +	if (unlikely((partial + len) >= SHA1_BLOCK_SIZE)) {
> +		int blocks;
> +
> +		if (partial) {
> +			int p = SHA1_BLOCK_SIZE - partial;
> +
> +			memcpy(sctx->buffer + partial, data, p);
> +			data += p;
> +			len -= p;
> +
> +			block_fn(sctx, sctx->buffer, 1);
> +		}
> +
> +		blocks = len / SHA1_BLOCK_SIZE;
> +		len %= SHA1_BLOCK_SIZE;
> +
> +		if (blocks) {
> +			block_fn(sctx, data, blocks);
> +			data += blocks * SHA1_BLOCK_SIZE;
> +		}
> +		partial = 0;
> +	}
> +	if (len)
> +		memcpy(sctx->buffer + partial, data, len);
> +
> +	return 0;
> +}
> +
> +static inline int sha1_base_do_finalize(struct sha1_state *sctx,
> +					sha1_block_fn *block_fn)
> +{
> +	const int bit_offset = SHA1_BLOCK_SIZE - sizeof(__be64);
> +	__be64 *bits = (__be64 *)(sctx->buffer + bit_offset);
> +	unsigned int partial = sctx->count % SHA1_BLOCK_SIZE;
> +
> +	sctx->buffer[partial++] = 0x80;
> +	if (partial > bit_offset) {
> +		memset(sctx->buffer + partial, 0x0, SHA1_BLOCK_SIZE - partial);
> +		partial = 0;
> +
> +		block_fn(sctx, sctx->buffer, 1);
> +	}
> +
> +	memset(sctx->buffer + partial, 0x0, bit_offset - partial);
> +	*bits = cpu_to_be64(sctx->count << 3);
> +	block_fn(sctx, sctx->buffer, 1);
> +
> +	return 0;
> +}
> +
> +static inline int sha1_base_finish(struct sha1_state *sctx, u8 *out)
> +{
> +	__be32 *digest = (__be32 *)out;
> +	int i;
> +
> +	for (i = 0; i < SHA1_DIGEST_SIZE / (int)sizeof(__be32); i++)
> +		put_unaligned_be32(sctx->state[i], digest++);
> +
> +	memzero_explicit(sctx, sizeof(*sctx));
> +	return 0;
> +}
> +
> +#endif /* _CRYPTO_SHA1_BASE_H */
> diff --git a/tools/perf/util/sha1_generic.c b/tools/perf/util/sha1_generic.c
> new file mode 100644
> index 000000000000..b0a7af370d59
> --- /dev/null
> +++ b/tools/perf/util/sha1_generic.c
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Cryptographic API.
> + *
> + * SHA1 Secure Hash Algorithm.
> + *
> + * Derived from cryptoapi implementation, adapted for in-place
> + * scatterlist interface.
> + *
> + * Copyright (c) Alan Smithee.
> + * Copyright (c) Andrew McDonald <andrew@mcdonald.org.uk>
> + * Copyright (c) Jean-Francois Dive <jef@linuxbe.org>
> + */
> +#include <linux/types.h>
> +#include <linux/string.h>
> +#include <asm/byteorder.h>
> +
> +#include "sha1_base.h"
> +
> +static void sha1_generic_block_fn(struct sha1_state *sst, u8 const *src,
> +				  int blocks)
> +{
> +	u32 temp[SHA1_WORKSPACE_WORDS];
> +
> +	while (blocks--) {
> +		sha1_transform(sst->state, (const char *)src, temp);
> +		src += SHA1_BLOCK_SIZE;
> +	}
> +	memzero_explicit(temp, sizeof(temp));
> +}
> +
> +int crypto_sha1_update(struct sha1_state *desc, const u8 *data,
> +		       unsigned int len)
> +{
> +	return sha1_base_do_update(desc, data, len, sha1_generic_block_fn);
> +}
> +
> +static int sha1_final(struct sha1_state *desc, u8 *out)
> +{
> +	sha1_base_do_finalize(desc, sha1_generic_block_fn);
> +	return sha1_base_finish(desc, out);
> +}
> +
> +int crypto_sha1_finup(struct sha1_state *desc, const u8 *data,
> +		      unsigned int len, u8 *out)
> +{
> +	sha1_base_do_update(desc, data, len, sha1_generic_block_fn);
> +	return sha1_final(desc, out);
> +}
> -- 
> 2.49.0.1164.gab81da1b16-goog

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 0/4] perf: Remove libcrypto dependency
  2025-05-21 22:53 [PATCH v1 0/4] perf: Remove libcrypto dependency Yuzhuo Jing
                   ` (3 preceding siblings ...)
  2025-05-21 22:53 ` [PATCH v1 4/4] tools: Remove libcrypto dependency Yuzhuo Jing
@ 2025-05-29 19:31 ` Ian Rogers
  2025-05-29 20:24   ` Arnaldo Carvalho de Melo
  4 siblings, 1 reply; 15+ messages in thread
From: Ian Rogers @ 2025-05-29 19:31 UTC (permalink / raw)
  To: Yuzhuo Jing
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Adrian Hunter, Liang Kan, Nathan Chancellor, Nick Desaulniers,
	Bill Wendling, Justin Stitt, Steven Rostedt (Google), James Clark,
	Tomas Glozar, Leo Yan, Guilherme Amadio, Yang Jihong,
	Masami Hiramatsu (Google), Adhemerval Zanella, Wei Yang,
	Ard Biesheuvel, Mike Rapoport (Microsoft), Athira Rajeev,
	Kajol Jain, Aditya Gupta, Charlie Jenkins, Steinar H. Gunderson,
	Dr. David Alan Gilbert, Herbert Xu, Jeff Johnson, Al Viro,
	linux-kernel, linux-perf-users, llvm

On Wed, May 21, 2025 at 3:54 PM Yuzhuo Jing <yuzhuo@google.com> wrote:
>
> Currently, genelf.c is the only file in the perf tool that depends on
> libcrypto (e.g. openssl), which only uses it to calculate a SHA1/MD5
> Build ID.  This patch series pulls in the SHA1 implementation from the
> kernel tree, and removes the libcrypto dependency from perf.  This also
> switches the default Build ID calculation method from MD5 to the more
> commonly used SHA1.
>
> Yuzhuo Jing (4):
>   perf utils: Add support functions for sha1 utils
>   perf tools: Add sha1 utils
>   perf genelf: Remove libcrypto dependency and use sha1 utils
>   tools: Remove libcrypto dependency

Tested-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

>  tools/build/Makefile.feature            |   2 -
>  tools/build/feature/Makefile            |   4 -
>  tools/build/feature/test-all.c          |   5 -
>  tools/build/feature/test-libcrypto.c    |  25 -----
>  tools/include/linux/bitops.h            |  10 ++
>  tools/include/linux/compiler.h          |  17 ++++
>  tools/include/linux/string.h            |  22 +++++
>  tools/perf/Documentation/perf-check.txt |   1 -
>  tools/perf/Makefile.config              |  13 ---
>  tools/perf/Makefile.perf                |   3 -
>  tools/perf/builtin-check.c              |   1 -
>  tools/perf/tests/make                   |   4 +-
>  tools/perf/util/Build                   |   2 +
>  tools/perf/util/genelf.c                |  72 ++------------
>  tools/perf/util/sha1.c                  | 122 ++++++++++++++++++++++++
>  tools/perf/util/sha1.h                  |  41 ++++++++
>  tools/perf/util/sha1_base.h             | 103 ++++++++++++++++++++
>  tools/perf/util/sha1_generic.c          |  49 ++++++++++
>  18 files changed, 373 insertions(+), 123 deletions(-)
>  delete mode 100644 tools/build/feature/test-libcrypto.c
>  create mode 100644 tools/perf/util/sha1.c
>  create mode 100644 tools/perf/util/sha1.h
>  create mode 100644 tools/perf/util/sha1_base.h
>  create mode 100644 tools/perf/util/sha1_generic.c
>
> --
> 2.49.0.1164.gab81da1b16-goog
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 0/4] perf: Remove libcrypto dependency
  2025-05-29 19:31 ` [PATCH v1 0/4] perf: " Ian Rogers
@ 2025-05-29 20:24   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-29 20:24 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Yuzhuo Jing, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Liang Kan, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
	Justin Stitt, Steven Rostedt (Google), James Clark, Tomas Glozar,
	Leo Yan, Guilherme Amadio, Yang Jihong, Masami Hiramatsu (Google),
	Adhemerval Zanella, Wei Yang, Ard Biesheuvel,
	Mike Rapoport (Microsoft), Athira Rajeev, Kajol Jain,
	Aditya Gupta, Charlie Jenkins, Steinar H. Gunderson,
	Dr. David Alan Gilbert, Herbert Xu, Jeff Johnson, Al Viro,
	linux-kernel, linux-perf-users, llvm

On Thu, May 29, 2025 at 12:31:59PM -0700, Ian Rogers wrote:
> On Wed, May 21, 2025 at 3:54 PM Yuzhuo Jing <yuzhuo@google.com> wrote:
> >
> > Currently, genelf.c is the only file in the perf tool that depends on
> > libcrypto (e.g. openssl), which only uses it to calculate a SHA1/MD5
> > Build ID.  This patch series pulls in the SHA1 implementation from the
> > kernel tree, and removes the libcrypto dependency from perf.  This also
> > switches the default Build ID calculation method from MD5 to the more
> > commonly used SHA1.
> >
> > Yuzhuo Jing (4):
> >   perf utils: Add support functions for sha1 utils
> >   perf tools: Add sha1 utils
> >   perf genelf: Remove libcrypto dependency and use sha1 utils
> >   tools: Remove libcrypto dependency
> 
> Tested-by: Ian Rogers <irogers@google.com>

I reported a problem with some integer comparision, the code is the same
as is in the kernel, so I left it for later to continue analysis, if
someone can try to continue from where I left, that could help.

But then this can be left for the v6.17 as we're already in the merge
window for v6.16 and we need to have some time for what is in
perf-tools-next to sit in linux-next before sending to Linus.

- Arnaldo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 2/4] perf tools: Add sha1 utils
  2025-05-22 17:56   ` Arnaldo Carvalho de Melo
@ 2025-06-04 18:17     ` Yuzhuo Jing
  2025-06-06 18:27       ` Ian Rogers
  0 siblings, 1 reply; 15+ messages in thread
From: Yuzhuo Jing @ 2025-06-04 18:17 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	Liang Kan, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
	Justin Stitt, Steven Rostedt (Google), James Clark, Tomas Glozar,
	Leo Yan, Guilherme Amadio, Yang Jihong, Masami Hiramatsu (Google),
	Adhemerval Zanella, Wei Yang, Ard Biesheuvel,
	Mike Rapoport (Microsoft), Athira Rajeev, Kajol Jain,
	Aditya Gupta, Charlie Jenkins, Steinar H. Gunderson,
	Dr. David Alan Gilbert, Herbert Xu, Jeff Johnson, Al Viro,
	linux-kernel, linux-perf-users, llvm

Hi Arnaldo,

Thanks for testing the patches!

> In file included from util/sha1_generic.c:18:
> util/sha1_base.h: In function ‘sha1_base_do_finalize’:
> util/sha1_base.h:77:21: error: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
>    77 |         if (partial > bit_offset) {
>       |                     ^
> cc1: all warnings being treated as errors

Oh, I didn't see that on my GCC 14.2.  A quick fix would work:

--- /dev/fd/63  2025-06-04 09:54:42.344516115 -0700
+++ tools/perf/util/sha1_base.h 2025-06-03 15:43:39.194036707 -0700
@@ -71,7 +69,7 @@
 static inline int sha1_base_do_finalize(struct sha1_state *sctx,
                                        sha1_block_fn *block_fn)
 {
-       const int bit_offset = SHA1_BLOCK_SIZE - sizeof(__be64);
+       const unsigned int bit_offset = SHA1_BLOCK_SIZE - sizeof(__be64);
        __be64 *bits = (__be64 *)(sctx->buffer + bit_offset);
        unsigned int partial = sctx->count % SHA1_BLOCK_SIZE;


To test it, I added -Werror=sign-compare to my local Makefile.config to
force the error.

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index d19d1f132726..9909611be301 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -225,9 +225,9 @@ endif

 # Treat warnings as errors unless directed not to
 ifneq ($(WERROR),0)
-  CORE_CFLAGS += -Werror
-  CXXFLAGS += -Werror
-  HOSTCFLAGS += -Werror
+  CORE_CFLAGS += -Werror=sign-compare -Werror
+  CXXFLAGS += -Werror=sign-compare -Werror
+  HOSTCFLAGS += -Werror=sign-compare -Werror
 endif

 ifndef DEBUG


While testing with "make -C tools/perf -f tests/make make_debug", I saw
similar compile errors in libjvmti.c:

jvmti/libjvmti.c: In function ‘copy_class_filename’:
jvmti/libjvmti.c:148:39: error: comparison of integer expressions of
different signedness: ‘int’ and ‘long unsigned int’
[-Werror=sign-compare]
  148 |                         for (i = 0; i < (size_t)(p - class_sign); i++)
      |                                       ^
jvmti/libjvmti.c:155:31: error: comparison of integer expressions of
different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’}
[-Werror=sign-compare]
  155 |                 for (j = 0; i < (max_length - 1) && file_name
&& j < strlen(file_name); j++, i++)
      |                               ^
jvmti/libjvmti.c:155:68: error: comparison of integer expressions of
different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’}
[-Werror=sign-compare]
  155 |                 for (j = 0; i < (max_length - 1) && file_name
&& j < strlen(file_name); j++, i++)
      |                                                                    ^

I've just sent a separate patch to the mailing list:
https://lore.kernel.org/lkml/20250604173632.2362759-1-yuzhuo@google.com/T/


> Humm that part is the same as in the kernel...
>
> ⬢ [acme@toolbx perf-tools-next]$ line=$(ctags -x --c-kinds=f include/crypto/sha1_base.h | awk '$1 == "sha1_base_do_finalize" {print $3}')
> ⬢ [acme@toolbx perf-tools-next]$ sed -n $line,\$p include/crypto/sha1_base.h | awk '{print} /\{/ {c++} /\}/ {c--; if (c==0) exit}' > /tmp/original
> ⬢ [acme@toolbx perf-tools-next]$ line=$(ctags -x --c-kinds=f tools/perf/util/sha1_base.h | awk '$1 == "sha1_base_do_finalize" {print $3}')
> ⬢ [acme@toolbx perf-tools-next]$ sed -n $line,\$p include/crypto/sha1_base.h | awk '{print} /\{/ {c++} /\}/ {c--; if (c==0) exit}' > /tmp/copy
> ⬢ [acme@toolbx perf-tools-next]$ diff -u /tmp/original /tmp/copy
> --- /tmp/original       2025-05-22 14:48:31.338406860 -0300
> +++ /tmp/copy   2025-05-22 14:48:58.401603439 -0300
> @@ -1,3 +1,7 @@
> +
> +       return 0;
> +}
> +
>  static inline int sha1_base_do_finalize(struct shash_desc *desc,
>                                         sha1_block_fn *block_fn)
>  {
> @@ -13,10 +17,3 @@
>
>                 block_fn(sctx, sctx->buffer, 1);
>         }
> -
> -       memset(sctx->buffer + partial, 0x0, bit_offset - partial);
> -       *bits = cpu_to_be64(sctx->count << 3);
> -       block_fn(sctx, sctx->buffer, 1);
> -
> -       return 0;
> -}
> ⬢ [acme@toolbx perf-tools-next]$

There were some other fixes that I made only to the perf tree version,
while maintaining verbatim for other parts.  Here's a script that
retains and compares only the copied parts.

  # Ignore everything after sha1_transform
  diff -u -B -I "^#include " <(sed -n
'/EXPORT_SYMBOL(sha1_transform)/q;p' lib/crypto/sha1.c)
tools/perf/util/sha1.c
  diff -u -B -I "^#include " -I "sha1_zero_message_hash" -I "^struct
sha1_state;$" -I "^void sha1_init(__u32 \*buf);$" \
      <(sed 's/shash_desc/sha1_state/g;' include/crypto/sha1.h)
tools/perf/util/sha1.h
  diff -u -B -I "^EXPORT_SYMBOL" -I "^#include " \
      <(sed 's/shash_desc \*desc/sha1_state *sctx/g;
/shash_desc_ctx(desc)/d' include/crypto/sha1_base.h)
tools/perf/util/sha1_base.h
  # Ignore everything after crypto_sha1_finup
  diff -u -B -I "^EXPORT_SYMBOL" -I "^#include " \
      <(sed -n -e '/const u8
sha1_zero_message_hash/,/EXPORT_SYMBOL_GPL(sha1_zero_message_hash)/d'
\
               -e 's/shash_desc/sha1_state/g;
/EXPORT_SYMBOL(crypto_sha1_finup)/q;p' crypto/sha1_generic.c) \
      tools/perf/util/sha1_generic.c

And the changes are as below (including the quick fix above), with one
changing the sign and integer type and another fixing type mismatch from
const u8 * to const char *.

Should we send another patch to the kernel tree version to fix the sign
error, or we add rules to allow perf tree only changes?

--- /dev/fd/63  2025-06-04 09:54:42.344516115 -0700
+++ tools/perf/util/sha1_base.h 2025-06-03 15:43:39.194036707 -0700
@@ -71,7 +69,7 @@
 static inline int sha1_base_do_finalize(struct sha1_state *sctx,
                                        sha1_block_fn *block_fn)
 {
-       const int bit_offset = SHA1_BLOCK_SIZE - sizeof(__be64);
+       const unsigned int bit_offset = SHA1_BLOCK_SIZE - sizeof(__be64);
        __be64 *bits = (__be64 *)(sctx->buffer + bit_offset);
        unsigned int partial = sctx->count % SHA1_BLOCK_SIZE;

@@ -95,7 +93,7 @@
        __be32 *digest = (__be32 *)out;
        int i;

-       for (i = 0; i < SHA1_DIGEST_SIZE / sizeof(__be32); i++)
+       for (i = 0; i < SHA1_DIGEST_SIZE / (int)sizeof(__be32); i++)
                put_unaligned_be32(sctx->state[i], digest++);

        memzero_explicit(sctx, sizeof(*sctx));
--- /dev/fd/63  2025-06-04 09:54:42.344516115 -0700
+++ tools/perf/util/sha1_generic.c      2025-05-16 10:52:59.219531034 -0700
@@ -27,7 +23,7 @@
        u32 temp[SHA1_WORKSPACE_WORDS];

        while (blocks--) {
-               sha1_transform(sst->state, src, temp);
+               sha1_transform(sst->state, (const char *)src, temp);
                src += SHA1_BLOCK_SIZE;
        }
        memzero_explicit(temp, sizeof(temp));

Thanks!

Best regards,
Yuzhuo

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 2/4] perf tools: Add sha1 utils
  2025-06-04 18:17     ` Yuzhuo Jing
@ 2025-06-06 18:27       ` Ian Rogers
  2025-06-06 20:17         ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 15+ messages in thread
From: Ian Rogers @ 2025-06-06 18:27 UTC (permalink / raw)
  To: Yuzhuo Jing, Herbert Xu, Eric Biggers
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Adrian Hunter, Liang Kan, Nathan Chancellor, Nick Desaulniers,
	Bill Wendling, Justin Stitt, Steven Rostedt (Google), James Clark,
	Tomas Glozar, Leo Yan, Guilherme Amadio, Yang Jihong,
	Masami Hiramatsu (Google), Adhemerval Zanella, Wei Yang,
	Ard Biesheuvel, Mike Rapoport (Microsoft), Athira Rajeev,
	Kajol Jain, Aditya Gupta, Charlie Jenkins, Steinar H. Gunderson,
	Dr. David Alan Gilbert, Jeff Johnson, Al Viro, linux-kernel,
	linux-perf-users, llvm

On Wed, Jun 4, 2025 at 11:17 AM Yuzhuo Jing <yuzhuo@google.com> wrote:
>
> Hi Arnaldo,
>
> Thanks for testing the patches!
>
> > In file included from util/sha1_generic.c:18:
> > util/sha1_base.h: In function ‘sha1_base_do_finalize’:
> > util/sha1_base.h:77:21: error: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
> >    77 |         if (partial > bit_offset) {
> >       |                     ^
> > cc1: all warnings being treated as errors
>
> Oh, I didn't see that on my GCC 14.2.  A quick fix would work:
>
> --- /dev/fd/63  2025-06-04 09:54:42.344516115 -0700
> +++ tools/perf/util/sha1_base.h 2025-06-03 15:43:39.194036707 -0700
> @@ -71,7 +69,7 @@
>  static inline int sha1_base_do_finalize(struct sha1_state *sctx,
>                                         sha1_block_fn *block_fn)
>  {
> -       const int bit_offset = SHA1_BLOCK_SIZE - sizeof(__be64);
> +       const unsigned int bit_offset = SHA1_BLOCK_SIZE - sizeof(__be64);
>         __be64 *bits = (__be64 *)(sctx->buffer + bit_offset);
>         unsigned int partial = sctx->count % SHA1_BLOCK_SIZE;
>
>
> To test it, I added -Werror=sign-compare to my local Makefile.config to
> force the error.
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index d19d1f132726..9909611be301 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -225,9 +225,9 @@ endif
>
>  # Treat warnings as errors unless directed not to
>  ifneq ($(WERROR),0)
> -  CORE_CFLAGS += -Werror
> -  CXXFLAGS += -Werror
> -  HOSTCFLAGS += -Werror
> +  CORE_CFLAGS += -Werror=sign-compare -Werror
> +  CXXFLAGS += -Werror=sign-compare -Werror
> +  HOSTCFLAGS += -Werror=sign-compare -Werror
>  endif
>
>  ifndef DEBUG
>
>
> While testing with "make -C tools/perf -f tests/make make_debug", I saw
> similar compile errors in libjvmti.c:
>
> jvmti/libjvmti.c: In function ‘copy_class_filename’:
> jvmti/libjvmti.c:148:39: error: comparison of integer expressions of
> different signedness: ‘int’ and ‘long unsigned int’
> [-Werror=sign-compare]
>   148 |                         for (i = 0; i < (size_t)(p - class_sign); i++)
>       |                                       ^
> jvmti/libjvmti.c:155:31: error: comparison of integer expressions of
> different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’}
> [-Werror=sign-compare]
>   155 |                 for (j = 0; i < (max_length - 1) && file_name
> && j < strlen(file_name); j++, i++)
>       |                               ^
> jvmti/libjvmti.c:155:68: error: comparison of integer expressions of
> different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’}
> [-Werror=sign-compare]
>   155 |                 for (j = 0; i < (max_length - 1) && file_name
> && j < strlen(file_name); j++, i++)
>       |                                                                    ^
>
> I've just sent a separate patch to the mailing list:
> https://lore.kernel.org/lkml/20250604173632.2362759-1-yuzhuo@google.com/T/

Thanks Yuzhuo! I guess this happened because jvmti.h is missing in the
test container. It seems to make sense to add -Wsign-compare to the
standard CFLAGS to lower the chance of breaking this container again.

> > Humm that part is the same as in the kernel...
> >
> > ⬢ [acme@toolbx perf-tools-next]$ line=$(ctags -x --c-kinds=f include/crypto/sha1_base.h | awk '$1 == "sha1_base_do_finalize" {print $3}')
> > ⬢ [acme@toolbx perf-tools-next]$ sed -n $line,\$p include/crypto/sha1_base.h | awk '{print} /\{/ {c++} /\}/ {c--; if (c==0) exit}' > /tmp/original
> > ⬢ [acme@toolbx perf-tools-next]$ line=$(ctags -x --c-kinds=f tools/perf/util/sha1_base.h | awk '$1 == "sha1_base_do_finalize" {print $3}')
> > ⬢ [acme@toolbx perf-tools-next]$ sed -n $line,\$p include/crypto/sha1_base.h | awk '{print} /\{/ {c++} /\}/ {c--; if (c==0) exit}' > /tmp/copy
> > ⬢ [acme@toolbx perf-tools-next]$ diff -u /tmp/original /tmp/copy
> > --- /tmp/original       2025-05-22 14:48:31.338406860 -0300
> > +++ /tmp/copy   2025-05-22 14:48:58.401603439 -0300
> > @@ -1,3 +1,7 @@
> > +
> > +       return 0;
> > +}
> > +
> >  static inline int sha1_base_do_finalize(struct shash_desc *desc,
> >                                         sha1_block_fn *block_fn)
> >  {
> > @@ -13,10 +17,3 @@
> >
> >                 block_fn(sctx, sctx->buffer, 1);
> >         }
> > -
> > -       memset(sctx->buffer + partial, 0x0, bit_offset - partial);
> > -       *bits = cpu_to_be64(sctx->count << 3);
> > -       block_fn(sctx, sctx->buffer, 1);
> > -
> > -       return 0;
> > -}
> > ⬢ [acme@toolbx perf-tools-next]$
>
> There were some other fixes that I made only to the perf tree version,
> while maintaining verbatim for other parts.  Here's a script that
> retains and compares only the copied parts.
>
>   # Ignore everything after sha1_transform
>   diff -u -B -I "^#include " <(sed -n
> '/EXPORT_SYMBOL(sha1_transform)/q;p' lib/crypto/sha1.c)
> tools/perf/util/sha1.c
>   diff -u -B -I "^#include " -I "sha1_zero_message_hash" -I "^struct
> sha1_state;$" -I "^void sha1_init(__u32 \*buf);$" \
>       <(sed 's/shash_desc/sha1_state/g;' include/crypto/sha1.h)
> tools/perf/util/sha1.h
>   diff -u -B -I "^EXPORT_SYMBOL" -I "^#include " \
>       <(sed 's/shash_desc \*desc/sha1_state *sctx/g;
> /shash_desc_ctx(desc)/d' include/crypto/sha1_base.h)
> tools/perf/util/sha1_base.h
>   # Ignore everything after crypto_sha1_finup
>   diff -u -B -I "^EXPORT_SYMBOL" -I "^#include " \
>       <(sed -n -e '/const u8
> sha1_zero_message_hash/,/EXPORT_SYMBOL_GPL(sha1_zero_message_hash)/d'
> \
>                -e 's/shash_desc/sha1_state/g;
> /EXPORT_SYMBOL(crypto_sha1_finup)/q;p' crypto/sha1_generic.c) \
>       tools/perf/util/sha1_generic.c
>
> And the changes are as below (including the quick fix above), with one
> changing the sign and integer type and another fixing type mismatch from
> const u8 * to const char *.
>
> Should we send another patch to the kernel tree version to fix the sign
> error, or we add rules to allow perf tree only changes?

I believe there will need to be a set of patches for the kernel sha1
code (fixing -Wsign-compare, any other typing issues) and a set of
patches for perf with the check-headers.sh updated as your scripts
check above. The perf patches shouldn't assume kernel patches have
landed. The perf check-headers.sh produces a warning but doesn't stop
the perf build. When the kernel changes land we can update the perf
check-headers.sh expectations.

Thanks,
Ian

> --- /dev/fd/63  2025-06-04 09:54:42.344516115 -0700
> +++ tools/perf/util/sha1_base.h 2025-06-03 15:43:39.194036707 -0700
> @@ -71,7 +69,7 @@
>  static inline int sha1_base_do_finalize(struct sha1_state *sctx,
>                                         sha1_block_fn *block_fn)
>  {
> -       const int bit_offset = SHA1_BLOCK_SIZE - sizeof(__be64);
> +       const unsigned int bit_offset = SHA1_BLOCK_SIZE - sizeof(__be64);
>         __be64 *bits = (__be64 *)(sctx->buffer + bit_offset);
>         unsigned int partial = sctx->count % SHA1_BLOCK_SIZE;
>
> @@ -95,7 +93,7 @@
>         __be32 *digest = (__be32 *)out;
>         int i;
>
> -       for (i = 0; i < SHA1_DIGEST_SIZE / sizeof(__be32); i++)
> +       for (i = 0; i < SHA1_DIGEST_SIZE / (int)sizeof(__be32); i++)
>                 put_unaligned_be32(sctx->state[i], digest++);
>
>         memzero_explicit(sctx, sizeof(*sctx));
> --- /dev/fd/63  2025-06-04 09:54:42.344516115 -0700
> +++ tools/perf/util/sha1_generic.c      2025-05-16 10:52:59.219531034 -0700
> @@ -27,7 +23,7 @@
>         u32 temp[SHA1_WORKSPACE_WORDS];
>
>         while (blocks--) {
> -               sha1_transform(sst->state, src, temp);
> +               sha1_transform(sst->state, (const char *)src, temp);
>                 src += SHA1_BLOCK_SIZE;
>         }
>         memzero_explicit(temp, sizeof(temp));
>
> Thanks!
>
> Best regards,
> Yuzhuo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v1 2/4] perf tools: Add sha1 utils
  2025-06-06 18:27       ` Ian Rogers
@ 2025-06-06 20:17         ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-06-06 20:17 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Yuzhuo Jing, Herbert Xu, Eric Biggers, Peter Zijlstra,
	Ingo Molnar, Namhyung Kim, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Adrian Hunter, Liang Kan, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt,
	Steven Rostedt (Google), James Clark, Tomas Glozar, Leo Yan,
	Guilherme Amadio, Yang Jihong, Masami Hiramatsu (Google),
	Adhemerval Zanella, Wei Yang, Ard Biesheuvel,
	Mike Rapoport (Microsoft), Athira Rajeev, Kajol Jain,
	Aditya Gupta, Charlie Jenkins, Steinar H. Gunderson,
	Dr. David Alan Gilbert, Jeff Johnson, Al Viro, linux-kernel,
	linux-perf-users, llvm

On Fri, Jun 06, 2025 at 11:27:28AM -0700, Ian Rogers wrote:
> On Wed, Jun 4, 2025 at 11:17 AM Yuzhuo Jing <yuzhuo@google.com> wrote:
> > Thanks for testing the patches!

You're welcome!

> > > In file included from util/sha1_generic.c:18:
> > > util/sha1_base.h: In function ‘sha1_base_do_finalize’:
> > > util/sha1_base.h:77:21: error: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
> > >    77 |         if (partial > bit_offset) {
> > >       |                     ^
> > > cc1: all warnings being treated as errors
> >
> > Oh, I didn't see that on my GCC 14.2.  A quick fix would work:
> >
> > --- /dev/fd/63  2025-06-04 09:54:42.344516115 -0700
> > +++ tools/perf/util/sha1_base.h 2025-06-03 15:43:39.194036707 -0700
> > @@ -71,7 +69,7 @@
> >  static inline int sha1_base_do_finalize(struct sha1_state *sctx,
> >                                         sha1_block_fn *block_fn)
> >  {
> > -       const int bit_offset = SHA1_BLOCK_SIZE - sizeof(__be64);
> > +       const unsigned int bit_offset = SHA1_BLOCK_SIZE - sizeof(__be64);
> >         __be64 *bits = (__be64 *)(sctx->buffer + bit_offset);
> >         unsigned int partial = sctx->count % SHA1_BLOCK_SIZE;
> >
> >
> > To test it, I added -Werror=sign-compare to my local Makefile.config to
> > force the error.
> >
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index d19d1f132726..9909611be301 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -225,9 +225,9 @@ endif
> >
> >  # Treat warnings as errors unless directed not to
> >  ifneq ($(WERROR),0)
> > -  CORE_CFLAGS += -Werror
> > -  CXXFLAGS += -Werror
> > -  HOSTCFLAGS += -Werror
> > +  CORE_CFLAGS += -Werror=sign-compare -Werror
> > +  CXXFLAGS += -Werror=sign-compare -Werror
> > +  HOSTCFLAGS += -Werror=sign-compare -Werror
> >  endif

> >  ifndef DEBUG

> > While testing with "make -C tools/perf -f tests/make make_debug", I saw
> > similar compile errors in libjvmti.c:

> > jvmti/libjvmti.c: In function ‘copy_class_filename’:
> > jvmti/libjvmti.c:148:39: error: comparison of integer expressions of
> > different signedness: ‘int’ and ‘long unsigned int’
> > [-Werror=sign-compare]
> >   148 |                         for (i = 0; i < (size_t)(p - class_sign); i++)
> >       |                                       ^
> > jvmti/libjvmti.c:155:31: error: comparison of integer expressions of
> > different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’}
> > [-Werror=sign-compare]
> >   155 |                 for (j = 0; i < (max_length - 1) && file_name
> > && j < strlen(file_name); j++, i++)
> >       |                               ^
> > jvmti/libjvmti.c:155:68: error: comparison of integer expressions of
> > different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’}
> > [-Werror=sign-compare]
> >   155 |                 for (j = 0; i < (max_length - 1) && file_name
> > && j < strlen(file_name); j++, i++)
> >       |                                                                    ^
> >
> > I've just sent a separate patch to the mailing list:
> > https://lore.kernel.org/lkml/20250604173632.2362759-1-yuzhuo@google.com/T/

> Thanks Yuzhuo! I guess this happened because jvmti.h is missing in the
> test container. It seems to make sense to add -Wsign-compare to the
> standard CFLAGS to lower the chance of breaking this container again.

> > > Humm that part is the same as in the kernel...
> > >
> > > ⬢ [acme@toolbx perf-tools-next]$ line=$(ctags -x --c-kinds=f include/crypto/sha1_base.h | awk '$1 == "sha1_base_do_finalize" {print $3}')
> > > ⬢ [acme@toolbx perf-tools-next]$ sed -n $line,\$p include/crypto/sha1_base.h | awk '{print} /\{/ {c++} /\}/ {c--; if (c==0) exit}' > /tmp/original
> > > ⬢ [acme@toolbx perf-tools-next]$ line=$(ctags -x --c-kinds=f tools/perf/util/sha1_base.h | awk '$1 == "sha1_base_do_finalize" {print $3}')
> > > ⬢ [acme@toolbx perf-tools-next]$ sed -n $line,\$p include/crypto/sha1_base.h | awk '{print} /\{/ {c++} /\}/ {c--; if (c==0) exit}' > /tmp/copy
> > > ⬢ [acme@toolbx perf-tools-next]$ diff -u /tmp/original /tmp/copy
> > > --- /tmp/original       2025-05-22 14:48:31.338406860 -0300
> > > +++ /tmp/copy   2025-05-22 14:48:58.401603439 -0300
> > > @@ -1,3 +1,7 @@
> > > +
> > > +       return 0;
> > > +}
> > > +
> > >  static inline int sha1_base_do_finalize(struct shash_desc *desc,
> > >                                         sha1_block_fn *block_fn)
> > >  {
> > > @@ -13,10 +17,3 @@
> > >
> > >                 block_fn(sctx, sctx->buffer, 1);
> > >         }
> > > -
> > > -       memset(sctx->buffer + partial, 0x0, bit_offset - partial);
> > > -       *bits = cpu_to_be64(sctx->count << 3);
> > > -       block_fn(sctx, sctx->buffer, 1);
> > > -
> > > -       return 0;
> > > -}
> > > ⬢ [acme@toolbx perf-tools-next]$
> >
> > There were some other fixes that I made only to the perf tree version,
> > while maintaining verbatim for other parts.  Here's a script that
> > retains and compares only the copied parts.
> >
> >   # Ignore everything after sha1_transform
> >   diff -u -B -I "^#include " <(sed -n
> > '/EXPORT_SYMBOL(sha1_transform)/q;p' lib/crypto/sha1.c)
> > tools/perf/util/sha1.c
> >   diff -u -B -I "^#include " -I "sha1_zero_message_hash" -I "^struct
> > sha1_state;$" -I "^void sha1_init(__u32 \*buf);$" \
> >       <(sed 's/shash_desc/sha1_state/g;' include/crypto/sha1.h)
> > tools/perf/util/sha1.h
> >   diff -u -B -I "^EXPORT_SYMBOL" -I "^#include " \
> >       <(sed 's/shash_desc \*desc/sha1_state *sctx/g;
> > /shash_desc_ctx(desc)/d' include/crypto/sha1_base.h)
> > tools/perf/util/sha1_base.h
> >   # Ignore everything after crypto_sha1_finup
> >   diff -u -B -I "^EXPORT_SYMBOL" -I "^#include " \
> >       <(sed -n -e '/const u8
> > sha1_zero_message_hash/,/EXPORT_SYMBOL_GPL(sha1_zero_message_hash)/d'
> > \
> >                -e 's/shash_desc/sha1_state/g;
> > /EXPORT_SYMBOL(crypto_sha1_finup)/q;p' crypto/sha1_generic.c) \
> >       tools/perf/util/sha1_generic.c
> >
> > And the changes are as below (including the quick fix above), with one
> > changing the sign and integer type and another fixing type mismatch from
> > const u8 * to const char *.
> >
> > Should we send another patch to the kernel tree version to fix the sign
> > error, or we add rules to allow perf tree only changes?
> 
> I believe there will need to be a set of patches for the kernel sha1
> code (fixing -Wsign-compare, any other typing issues) and a set of
> patches for perf with the check-headers.sh updated as your scripts
> check above.

Right, we try to reuse bits from the kernel as there are more people
working there and its a more critical piece of software than tooling
like perf, so when we notice something in the copies we carry, we better
fix it first in the origin.

> The perf patches shouldn't assume kernel patches have
> landed. The perf check-headers.sh produces a warning but doesn't stop
> the perf build. When the kernel changes land we can update the perf
> check-headers.sh expectations.

Right,

Thanks,

- Arnaldo
 
> Thanks,
> Ian
> 
> > --- /dev/fd/63  2025-06-04 09:54:42.344516115 -0700
> > +++ tools/perf/util/sha1_base.h 2025-06-03 15:43:39.194036707 -0700
> > @@ -71,7 +69,7 @@
> >  static inline int sha1_base_do_finalize(struct sha1_state *sctx,
> >                                         sha1_block_fn *block_fn)
> >  {
> > -       const int bit_offset = SHA1_BLOCK_SIZE - sizeof(__be64);
> > +       const unsigned int bit_offset = SHA1_BLOCK_SIZE - sizeof(__be64);
> >         __be64 *bits = (__be64 *)(sctx->buffer + bit_offset);
> >         unsigned int partial = sctx->count % SHA1_BLOCK_SIZE;
> >
> > @@ -95,7 +93,7 @@
> >         __be32 *digest = (__be32 *)out;
> >         int i;
> >
> > -       for (i = 0; i < SHA1_DIGEST_SIZE / sizeof(__be32); i++)
> > +       for (i = 0; i < SHA1_DIGEST_SIZE / (int)sizeof(__be32); i++)
> >                 put_unaligned_be32(sctx->state[i], digest++);
> >
> >         memzero_explicit(sctx, sizeof(*sctx));
> > --- /dev/fd/63  2025-06-04 09:54:42.344516115 -0700
> > +++ tools/perf/util/sha1_generic.c      2025-05-16 10:52:59.219531034 -0700
> > @@ -27,7 +23,7 @@
> >         u32 temp[SHA1_WORKSPACE_WORDS];
> >
> >         while (blocks--) {
> > -               sha1_transform(sst->state, src, temp);
> > +               sha1_transform(sst->state, (const char *)src, temp);
> >                 src += SHA1_BLOCK_SIZE;
> >         }
> >         memzero_explicit(temp, sizeof(temp));
> >
> > Thanks!
> >
> > Best regards,
> > Yuzhuo

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2025-06-06 20:17 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-21 22:53 [PATCH v1 0/4] perf: Remove libcrypto dependency Yuzhuo Jing
2025-05-21 22:53 ` [PATCH v1 1/4] perf utils: Add support functions for sha1 utils Yuzhuo Jing
2025-05-21 22:53 ` [PATCH v1 2/4] perf tools: Add " Yuzhuo Jing
2025-05-22 17:03   ` Arnaldo Carvalho de Melo
2025-05-22 17:56   ` Arnaldo Carvalho de Melo
2025-06-04 18:17     ` Yuzhuo Jing
2025-06-06 18:27       ` Ian Rogers
2025-06-06 20:17         ` Arnaldo Carvalho de Melo
2025-05-21 22:53 ` [PATCH v1 3/4] perf genelf: Remove libcrypto dependency and use " Yuzhuo Jing
2025-05-22 17:05   ` Arnaldo Carvalho de Melo
2025-05-22 17:23     ` Arnaldo Carvalho de Melo
2025-05-21 22:53 ` [PATCH v1 4/4] tools: Remove libcrypto dependency Yuzhuo Jing
2025-05-22 17:30   ` Arnaldo Carvalho de Melo
2025-05-29 19:31 ` [PATCH v1 0/4] perf: " Ian Rogers
2025-05-29 20:24   ` Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).