* [PATCH 1/3] profiling: fix sampling step on 64-bit big-endian systems
@ 2026-10-21 0:51 Andrew Morton
2026-10-21 0:52 ` [PATCH 2/3] profile: account for partial /proc/profile reads Andrew Morton
2026-10-21 0:52 ` [PATCH 3/3] profile: simplify /proc/profile read layout Andrew Morton
0 siblings, 2 replies; 4+ messages in thread
From: Andrew Morton @ 2026-10-21 0:51 UTC (permalink / raw)
To: brads, thnkslprpt, akpm, linux-kernel
From: Avi Weiss <thnkslprpt@gmail.com>
Subject: profiling: fix sampling step on 64-bit big-endian systems
Date: Tue, 20 Oct 2026 13:12:24 +0300
read_profile() stores the sampling step in an unsigned long but copies
only four bytes to userspace. On 64-bit big-endian systems (not common
obviously), this copies the upper 32 bits and discards the rest, causing
/proc/profile to report zero for normal shift values.
Cast to unsigned int so we get the correct 4 bytes, while keeping the
shift calculation in unsigned long.
A small C reproducer under qemu-s390x reproduced this and proved the fix
works for prof_shift=2.
Link: https://lore.kernel.org/20261020101225.917959-1-thnkslprpt@gmail.com
Fixes: 2d186afd04d6 ("profiling: fix shift-out-of-bounds bugs")
Signed-off-by: Avi Weiss <thnkslprpt@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Reviewed-by: Bradley Morgan <brads@mainlining.org>
---
kernel/profile.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/kernel/profile.c~profiling-fix-sampling-step-on-64-bit-big-endian-systems
+++ a/kernel/profile.c
@@ -156,7 +156,7 @@ read_profile(struct file *file, char __u
unsigned long p = *ppos;
ssize_t read;
char *pnt;
- unsigned long sample_step = 1UL << prof_shift;
+ unsigned int sample_step = (unsigned int)(1UL << prof_shift);
if (p >= (prof_len+1)*sizeof(unsigned int))
return 0;
_
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 2/3] profile: account for partial /proc/profile reads 2026-10-21 0:51 [PATCH 1/3] profiling: fix sampling step on 64-bit big-endian systems Andrew Morton @ 2026-10-21 0:52 ` Andrew Morton 2026-10-21 0:52 ` [PATCH 3/3] profile: simplify /proc/profile read layout Andrew Morton 1 sibling, 0 replies; 4+ messages in thread From: Andrew Morton @ 2026-10-21 0:52 UTC (permalink / raw) To: thnkslprpt, brads, akpm, linux-kernel From: Andrew Morton <akpm@linux-foundation.org> Subject: profile: account for partial /proc/profile reads Date: Tue, 20 Oct 2026 15:28:00 -0700 read_profile() returns -EFAULT without advancing the file position when a user copy faults, even if earlier header bytes or a prefix of the counters were successfully copied. A retry can therefore copy the same bytes again despite the previous read having delivered data. Return the number of successfully copied bytes and advance *ppos by that amount. Return -EFAULT only if the fault occurs before any bytes are copied. Preserve the four-byte sampling-step header, raw counter representation, and normal successful-read behavior. Avi Weiss's "profiling: fix sampling step on 64-bit big-endian systems" is unchanged. This patch changes only short-read and fault reporting, not the output format. Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Cc: Avi Weiss <thnkslprpt@gmail.com> Cc: Bradley Morgan <brads@mainlining.org> --- Tested the combined three-patch series on x86-64 QEMU/KVM with CONFIG_PROFILING=y, CONFIG_PROC_FS=y and profile=12. profile-read-test.c: PASS (0 failures). Covered header decoding, short and boundary reads, EOF, partial user-copy faults, return values and file-position accounting. The 64-bit big-endian fix was not runtime tested. There was no individual-patch or unpatched-versus-patched runtime comparison. kernel/profile.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) --- a/kernel/profile.c~profile-account-for-partial-proc-profile-reads +++ a/kernel/profile.c @@ -157,6 +157,7 @@ read_profile(struct file *file, char __u ssize_t read; char *pnt; unsigned int sample_step = (unsigned int)(1UL << prof_shift); + size_t not_copied; if (p >= (prof_len+1)*sizeof(unsigned int)) return 0; @@ -166,15 +167,21 @@ read_profile(struct file *file, char __u while (p < sizeof(unsigned int) && count > 0) { if (put_user(*((char *)(&sample_step)+p), buf)) - return -EFAULT; + goto out; buf++; p++; count--; read++; } - pnt = (char *)prof_buffer + p - sizeof(atomic_t); - if (copy_to_user(buf, (void *)pnt, count)) - return -EFAULT; - read += count; + if (count) { + pnt = (char *)prof_buffer + p - sizeof(atomic_t); + not_copied = copy_to_user(buf, pnt, count); + read += count - not_copied; + if (not_copied) + goto out; + } *ppos += read; return read; +out: + *ppos += read; + return read ? read : -EFAULT; } /* default is to not implement this call */ _ ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 3/3] profile: simplify /proc/profile read layout 2026-10-21 0:51 [PATCH 1/3] profiling: fix sampling step on 64-bit big-endian systems Andrew Morton 2026-10-21 0:52 ` [PATCH 2/3] profile: account for partial /proc/profile reads Andrew Morton @ 2026-10-21 0:52 ` Andrew Morton 2026-10-21 1:27 ` Andrew Morton 1 sibling, 1 reply; 4+ messages in thread From: Andrew Morton @ 2026-10-21 0:52 UTC (permalink / raw) To: thnkslprpt, brads, akpm, linux-kernel From: Andrew Morton <akpm@linux-foundation.org> Subject: profile: simplify /proc/profile read layout Date: Tue, 20 Oct 2026 23:58:33 +0000 read_profile() is needlessly difficult to understand. It copies the four-byte sampling-step header one byte at a time, then manufactures a pointer before prof_buffer so a file offset can be used as a counter-array offset. Its signed byte counter, unsigned long file offset and repeated size calculations further obscure where each read starts and how many bytes it transfers. Treat the file as two consecutive regions: a four-byte header and the profiling counters. Use size_t for byte positions and lengths, copy the bounded header in one operation, and address the counters relative to the start of prof_buffer. Brief comments explain each region. Keep the word-sized shift and explicit narrowing required by Avi Weiss's "profiling: fix sampling step on 64-bit big-endian systems". Assign the header value separately and explain why the 32-bit ABI requires a narrowing conversion. Preserve the partial-read and fault semantics established by [1/2], including short copies within either region. No userspace-visible behavior change is intended. Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Cc: Avi Weiss <thnkslprpt@gmail.com> Cc: Bradley Morgan <brads@mainlining.org> --- Tested the combined three-patch series on x86-64 QEMU/KVM, with CONFIG_PROFILING=y, CONFIG_PROC_FS=y and profile=12. profile-read-test.c: PASS (0 failures). Covered header decoding, short/unaligned and boundary reads, EOF, partial copy faults, return values and file-position updates. The 64-bit big-endian case was not tested. No per-patch or unpatched-versus-patched runtime comparison was reported. kernel/profile.c | 57 +++++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 22 deletions(-) --- a/kernel/profile.c~profile-simplify-proc-profile-read-layout +++ a/kernel/profile.c @@ -153,35 +153,48 @@ void profile_tick(int type) static ssize_t read_profile(struct file *file, char __user *buf, size_t count, loff_t *ppos) { - unsigned long p = *ppos; - ssize_t read; - char *pnt; - unsigned int sample_step = (unsigned int)(1UL << prof_shift); - size_t not_copied; + const size_t header_size = sizeof(unsigned int); + const size_t file_size = (prof_len + 1) * sizeof(unsigned int); + unsigned int sample_step; + size_t pos = *ppos; + size_t done = 0; + size_t n, not_copied; - if (p >= (prof_len+1)*sizeof(unsigned int)) + /* Keep the shift word-sized before narrowing to the 32-bit ABI. */ + sample_step = (unsigned int)(1UL << prof_shift); + + if (pos >= file_size) return 0; - if (count > (prof_len+1)*sizeof(unsigned int) - p) - count = (prof_len+1)*sizeof(unsigned int) - p; - read = 0; + count = min(count, file_size - pos); - while (p < sizeof(unsigned int) && count > 0) { - if (put_user(*((char *)(&sample_step)+p), buf)) - goto out; - buf++; p++; count--; read++; + /* Copy the four-byte sampling resolution. */ + if (pos < header_size) { + n = min(count, header_size - pos); + not_copied = copy_to_user(buf, (u8 *)&sample_step + pos, n); + done += n - not_copied; + if (not_copied) + goto fault; + buf += n; + pos += n; + count -= n; } + + /* Copy raw counters using offsets relative to the counter array. */ if (count) { - pnt = (char *)prof_buffer + p - sizeof(atomic_t); - not_copied = copy_to_user(buf, pnt, count); - read += count - not_copied; + not_copied = copy_to_user(buf, + (u8 *)prof_buffer + pos - header_size, + count); + done += count - not_copied; if (not_copied) - goto out; + goto fault; } - *ppos += read; - return read; -out: - *ppos += read; - return read ? read : -EFAULT; + *ppos += done; + return done; +fault: + *ppos += done; + if (!done) + return -EFAULT; + return done; } /* default is to not implement this call */ _ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 3/3] profile: simplify /proc/profile read layout 2026-10-21 0:52 ` [PATCH 3/3] profile: simplify /proc/profile read layout Andrew Morton @ 2026-10-21 1:27 ` Andrew Morton 0 siblings, 0 replies; 4+ messages in thread From: Andrew Morton @ 2026-10-21 1:27 UTC (permalink / raw) To: thnkslprpt, brads, linux-kernel On Tue, 20 Oct 2026 17:52:03 -0700 Andrew Morton <akpm@linux-foundation.org> wrote: > read_profile() is needlessly difficult to understand. It copies the > four-byte sampling-step header one byte at a time, then manufactures > a pointer before prof_buffer so a file offset can be used as a > counter-array offset. Its signed byte counter, unsigned long file > offset and repeated size calculations further obscure where each > read starts and how many bytes it transfers. > > Treat the file as two consecutive regions: a four-byte header and > the profiling counters. Use size_t for byte positions and lengths, > copy the bounded header in one operation, and address the counters > relative to the start of prof_buffer. Brief comments explain each > region. > > Keep the word-sized shift and explicit narrowing required by Avi > Weiss's "profiling: fix sampling step on 64-bit big-endian systems". > Assign the header value separately and explain why the 32-bit ABI > requires a narrowing conversion. > > Preserve the partial-read and fault semantics established by [1/2], > including short copies within either region. No userspace-visible > behavior change is intended. Me:0, Sashiko:1 revised version, I'll resend in a few days. I tested 32-bit now. From: Andrew Morton <akpm@linux-foundation.org> Subject: profile: simplify /proc/profile read layout Date: Tue, 20 Oct 2026 23:58:33 +0000 read_profile() is needlessly difficult to understand. It copies the four-byte sampling-step header one byte at a time, then manufactures a pointer before prof_buffer so a file offset can be used as a counter-array offset. Its signed byte counter, unsigned long file offset and repeated size calculations further obscure where each read starts and how many bytes it transfers. Treat the file as two consecutive regions: a four-byte header and the profiling counters. Use size_t for byte positions and lengths, copy the bounded header in one operation, and address the counters relative to the start of prof_buffer. Retain the full loff_t value for EOF and negative-offset checks before narrowing it to size_t. Brief comments explain each region. Keep the word-sized shift and explicit narrowing required by Avi Weiss's "profiling: fix sampling step on 64-bit big-endian systems". Assign the header value separately and explain why the 32-bit ABI requires a narrowing conversion. Preserve the partial-read and fault semantics established by [1/2], including short copies within either region. The full-width offset check also prevents 32-bit pread64() offsets from wrapping into earlier profile data. No other userspace-visible behavior change is intended. Signed-off-by: Andrew Morton <akpm@linux-foundation.org> --- Tested the combined three-patch series on x86-64 and i686 QEMU/KVM guests with CONFIG_PROFILING=y and profile=12. profile-read-test.c: PASS (0 failures) on both architectures. Covered short and unaligned reads, header boundaries, EOF, partial user-copy faults, return values, file positions and retry behavior. The 64-bit big-endian sampling-step fix was not runtime tested. No separate per-patch or unpatched-versus-patched run was reported. kernel/profile.c | 61 ++++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 22 deletions(-) --- a/kernel/profile.c~profile-simplify-proc-profile-read-layout +++ a/kernel/profile.c @@ -153,35 +153,52 @@ void profile_tick(int type) static ssize_t read_profile(struct file *file, char __user *buf, size_t count, loff_t *ppos) { - unsigned long p = *ppos; - ssize_t read; - char *pnt; - unsigned int sample_step = (unsigned int)(1UL << prof_shift); - size_t not_copied; + const size_t header_size = sizeof(unsigned int); + const size_t file_size = (prof_len + 1) * sizeof(unsigned int); + unsigned int sample_step; + loff_t offset = *ppos; + size_t pos; + size_t done = 0; + size_t n, not_copied; - if (p >= (prof_len+1)*sizeof(unsigned int)) + /* Keep the shift word-sized before narrowing to the 32-bit ABI. */ + sample_step = (unsigned int)(1UL << prof_shift); + + if (offset < 0) + return -EINVAL; + if (offset >= file_size) return 0; - if (count > (prof_len+1)*sizeof(unsigned int) - p) - count = (prof_len+1)*sizeof(unsigned int) - p; - read = 0; + pos = offset; + count = min(count, file_size - pos); - while (p < sizeof(unsigned int) && count > 0) { - if (put_user(*((char *)(&sample_step)+p), buf)) - goto out; - buf++; p++; count--; read++; + /* Copy the four-byte sampling resolution. */ + if (pos < header_size) { + n = min(count, header_size - pos); + not_copied = copy_to_user(buf, (u8 *)&sample_step + pos, n); + done += n - not_copied; + if (not_copied) + goto fault; + buf += n; + pos += n; + count -= n; } + + /* Copy raw counters using offsets relative to the counter array. */ if (count) { - pnt = (char *)prof_buffer + p - sizeof(atomic_t); - not_copied = copy_to_user(buf, pnt, count); - read += count - not_copied; + not_copied = copy_to_user(buf, + (u8 *)prof_buffer + pos - header_size, + count); + done += count - not_copied; if (not_copied) - goto out; + goto fault; } - *ppos += read; - return read; -out: - *ppos += read; - return read ? read : -EFAULT; + *ppos += done; + return done; +fault: + *ppos += done; + if (!done) + return -EFAULT; + return done; } /* default is to not implement this call */ _ ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-21 1:27 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-10-21 0:51 [PATCH 1/3] profiling: fix sampling step on 64-bit big-endian systems Andrew Morton 2026-10-21 0:52 ` [PATCH 2/3] profile: account for partial /proc/profile reads Andrew Morton 2026-10-21 0:52 ` [PATCH 3/3] profile: simplify /proc/profile read layout Andrew Morton 2026-10-21 1:27 ` Andrew Morton
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®