| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CADQikVDs3msXqZ6tyoXR0WeN4D_9snxWyk2kXeDw5iAs+BHFuw@mail.gmail.com>
Date: Wed, 4 Jun 2025 11:17:20 -0700
From: Yuzhuo Jing <yuzhuo@...gle.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Ian Rogers <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>,
Liang Kan <kan.liang@...ux.intel.com>, Nathan Chancellor <nathan@...nel.org>,
Nick Desaulniers <nick.desaulniers+lkml@...il.com>, Bill Wendling <morbo@...gle.com>,
Justin Stitt <justinstitt@...gle.com>, "Steven Rostedt (Google)" <rostedt@...dmis.org>,
James Clark <james.clark@...aro.org>, Tomas Glozar <tglozar@...hat.com>, Leo Yan <leo.yan@....com>,
Guilherme Amadio <amadio@...too.org>, Yang Jihong <yangjihong@...edance.com>,
"Masami Hiramatsu (Google)" <mhiramat@...nel.org>, Adhemerval Zanella <adhemerval.zanella@...aro.org>,
Wei Yang <richard.weiyang@...il.com>, Ard Biesheuvel <ardb@...nel.org>,
"Mike Rapoport (Microsoft)" <rppt@...nel.org>, Athira Rajeev <atrajeev@...ux.vnet.ibm.com>,
Kajol Jain <kjain@...ux.ibm.com>, Aditya Gupta <adityag@...ux.ibm.com>,
Charlie Jenkins <charlie@...osinc.com>, "Steinar H. Gunderson" <sesse@...gle.com>,
"Dr. David Alan Gilbert" <linux@...blig.org>, Herbert Xu <herbert@...dor.apana.org.au>,
Jeff Johnson <jeff.johnson@....qualcomm.com>, Al Viro <viro@...iv.linux.org.uk>,
linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
llvm@...ts.linux.dev
Subject: Re: [PATCH v1 2/4] perf tools: Add sha1 utils
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@...lbx perf-tools-next]$ line=$(ctags -x --c-kinds=f include/crypto/sha1_base.h | awk '$1 == "sha1_base_do_finalize" {print $3}')
> ⬢ [acme@...lbx perf-tools-next]$ sed -n $line,\$p include/crypto/sha1_base.h | awk '{print} /\{/ {c++} /\}/ {c--; if (c==0) exit}' > /tmp/original
> ⬢ [acme@...lbx perf-tools-next]$ line=$(ctags -x --c-kinds=f tools/perf/util/sha1_base.h | awk '$1 == "sha1_base_do_finalize" {print $3}')
> ⬢ [acme@...lbx perf-tools-next]$ sed -n $line,\$p include/crypto/sha1_base.h | awk '{print} /\{/ {c++} /\}/ {c--; if (c==0) exit}' > /tmp/copy
> ⬢ [acme@...lbx 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@...lbx 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
Powered by blists - more mailing lists