mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
To: rostedt@goodmis.org, mhiramat@kernel.org
Cc: mathieu.desnoyers@efficios.com, namhyung@kernel.org,
	zanussi@kernel.org, linux-trace-kernel@vger.kernel.org,
	linux-kernel@vger.kernel.org, donggeunyoo.kernel@gmail.com,
	sashiko-bot@kernel.org, stable@vger.kernel.org
Subject: [PATCH] tracing: Fix 64-bit and signed histogram fields on 32-bit kernels
Date: Thu, 17 Sep 2026 10:55:32 +0900	[thread overview]
Message-ID: <20260917015532.103081-1-donggeunyoo.kernel@gmail.com> (raw)

DEFINE_HIST_FIELD_FN() reads a trace event field and widens it to the u64
the histogram uses as a key or a value:

	type *addr = (type *)(event + hist_field->field->offset);

	return (u64)(unsigned long)*addr;

unsigned long is 32 bits on a 32-bit kernel, so the intermediate cast
discards the upper half of a 64-bit field, and widens a signed field by
zero extension from 32 bits rather than sign extension from its own
width. Five of the eight instantiations are wrong there: s64, u64, s32,
s16 and s8.

The cast came in with commit 79e577cbce4c ("tracing: Support string type
key properly"), which also added hist_field_dynstring() and
hist_field_pstring(). Those convert a char * to u64 and do warn without
an unsigned long in between on 32-bit. DEFINE_HIST_FIELD_FN() converts an
integer and never a pointer, so it gains nothing from the cast and only
loses information.

Drop the intermediate cast and let the usual arithmetic conversions widen
*addr according to its own type. unsigned long is 64 bits on LP64, so the
two forms are identical there and 64-bit kernels are unaffected.

Cc: stable@vger.kernel.org
Fixes: 79e577cbce4c ("tracing: Support string type key properly")
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/all/20260914054614.82A9B1F000FF@smtp.kernel.org/
Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
---
Tested under QEMU on v7.3-rc3-82-g238650ef6c7c, four arms: i386 and
x86_64, each unfixed and fixed, defconfig plus FTRACE, HIST_TRIGGERS,
USER_EVENTS and DEVTMPFS.  A user_events record declares one field of
each of the eight types DEFINE_HIST_FIELD_FN() instantiates and
userspace writes known values -- s64 -4294967396, u64 0x123456789abc,
s32/s16/s8 -100, u32 4294967295, u16 65535, u8 255 -- so every
instantiation is exercised by one record with no timing dependence.  The
eight fields are read back both as histogram keys and as summed values.

i386 before, keys and values agreeing:

  { s64v: 4294967196, u64v: 1450744508, s32v: 4294967196 } hitcount: 1
  { u32v: 4294967295, s16v: 4294967196, u16v:      65535 } hitcount: 1
  { s8v: 4294967196, u8v:        255 } hitcount: 1
  { common_pid: 1 } hitcount: 1  s64v: 4294967196  u64v: 1450744508
  { common_pid: 1 } hitcount: 1  s32v: 4294967196  u32v: 4294967295
  { common_pid: 1 } hitcount: 1  s16v: 4294967196  u16v:      65535
  { common_pid: 1 } hitcount: 1  s8v: 4294967196  u8v:        255

i386 after, byte-identical to x86_64 both before and after:

  { s64v: 18446744069414584220, u64v: 20015998343868, s32v: 18446744073709551516 } hitcount: 1
  { u32v: 4294967295, s16v: 18446744073709551516, u16v:      65535 } hitcount: 1
  { s8v: 18446744073709551516, u8v:        255 } hitcount: 1
  { common_pid: 1 } hitcount: 1  s64v: 18446744069414584220  u64v: 20015998343868
  { common_pid: 1 } hitcount: 1  s32v: 18446744073709551516  u32v: 4294967295
  { common_pid: 1 } hitcount: 1  s16v: 18446744073709551516  u16v:      65535
  { common_pid: 1 } hitcount: 1  s8v: 18446744073709551516  u8v:        255

Five of the eight move: s64, u64, s32, s16 and s8.  u32, u16 and u8 are
identical in all four arms.

trace_events_hist.o is byte-identical between the two x86_64 arms and
differs between the two i386 arms.

 kernel/trace/trace_events_hist.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 8af97fd4ee2d..9079b81cb7b1 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -466,7 +466,7 @@ static u64 hist_field_unary_minus(struct hist_field *hist_field,
 {									\
 	type *addr = (type *)(event + hist_field->field->offset);	\
 									\
-	return (u64)(unsigned long)*addr;				\
+	return (u64)*addr;						\
 }
 
 DEFINE_HIST_FIELD_FN(s64);
-- 
2.53.0


                 reply	other threads:[~2026-09-17  1:55 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260917015532.103081-1-donggeunyoo.kernel@gmail.com \
    --to=donggeunyoo.kernel@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=sashiko-bot@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=zanussi@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®