Re: [PATCH 05/44] ipc/msg: Fix saturation of percpu counts in msgctl_info()

[Andrew Morton]

On Thu, 29 Oct 2026 09:45:43 +0000 David Laight <david.laight.linux@xxxxxxxxx> wrote:

> On Wed, 28 Oct 2026 21:04:54 -0700
> Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> 
> > On Wed, 28 Oct 2026 18:15:02 -0700 Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> > 
> > > > diff --git a/ipc/msg.c b/ipc/msg.c
> > > > index ee6af4fe52bf..34e607add006 100644
> > > > --- a/ipc/msg.c
> > > > +++ b/ipc/msg.c
> > > > @@ -501,11 +501,9 @@ static int msgctl_info(struct ipc_namespace *ns, int msqid,
> > > >  	max_idx = ipc_get_maxidx(&msg_ids(ns));
> > > >  	up_read(&msg_ids(ns).rwsem);
> > > >  	if (cmd == MSG_INFO) {
> > > > -		msginfo->msgmap = min_t(int,
> > > > -				     percpu_counter_sum(&ns->percpu_msg_hdrs),
> > > > +		msginfo->msgmap = min(percpu_counter_sum(&ns->percpu_msg_hdrs),
> > > >  				     INT_MAX);
> > > > -		msginfo->msgtql = min_t(int,
> > > > -		                     percpu_counter_sum(&ns->percpu_msg_bytes),
> > > > +		msginfo->msgtql = min(percpu_counter_sum(&ns->percpu_msg_bytes),
> > > >  				     INT_MAX);
> > > >  	} else {
> > > >  		msginfo->msgmap = MSGMAP;  
> > > 
> > > Added, thanks.
> > >   
> > 
> > LLM found another bug in there.  Please review:
> 
> Seems reasonable, if unlikely to ever happen.
> Description is overlong ...
> 
> Might be worth doing a scan of all uses of percpu_counter_sum(),
> I can imagine it is a common problem.

I had LLM do a scan:

: The scan found four similar reporting problems:
: 
: - Ceph: metrics_file_show() in fs/ceph/debugfs.c prints the raw
:   opened-inode count as signed. ceph_mdsc_send_metrics() in
:   fs/ceph/metric.c converts it to unsigned for the MDS. An open/close
:   pair on different CPUs can yield -1, appearing as a negative count
:   in debugfs or U64_MAX in the metric.
: 
: - ext4: ext4_attr_show() in fs/ext4/sysfs.c uses the raw dirty-cluster
:   count for delayed_allocation_blocks. Reserving and then consuming
:   or releasing a reservation on different CPUs can produce a
:   negative sample. With one block per cluster, -1 is reported as
:   U64_MAX blocks.
: 
: - BPF: htab_map_mem_usage() in kernel/bpf/hashtab.c converts the raw
:   entry count to unsigned before calculating memory usage. This
:   affects nonpreallocated hash maps using per-CPU counters. An
:   insert/delete pair can yield -1 entries and an incorrect memory
:   estimate, including less than the empty-map overhead.
: 
: - tmpfs: shmem_statfs() in mm/shmem.c subtracts the raw used-block
:   count from the block limit. An allocation/free pair can yield -1
:   used blocks, reporting more free blocks than total blocks. Tools
:   calculating used space as total minus free can consequently
:   report negative usage.
: 
: These are source-level findings, not runtime reproductions.  No
: corruption or resource-limit bypass was established.
: 
: Other callers already floor negative samples, count cumulative events,
: check accounting after teardown, or use deliberately signed values. 
: Drain checks and some XFS statistics still need further investigation. 
: This should not be a blanket conversion, and flooring negative samples
: does not make the statistics an atomic snapshot.

> Oh and the code could use clamp(per_cpu_counter_sum(...), 0, INT_MAX);
> which is exactly the same but maybe clearer?

Sounds good.  How does the below look?


From: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Subject: ipc/msg: clamp negative MSG_INFO usage counts
Date: Thu, 29 Oct 2026 01:43:24 +0000

MSG_INFO can report negative usage during ordinary queue traffic.  For
example, ipcs -q -u prints negative header counts and converts negative
byte counts into enormous unsigned used-space values.

percpu_counter_sum() can sample the sending CPU before an enqueue and the
receiving CPU after the matching dequeue, yielding a negative total. 
Clamp both counts to [0, INT_MAX] before converting to int.  The
statistics remain an approximate, non-atomic snapshot.

Fixes: 72d1e611082e ("ipc/msg: mitigate the lock contention with percpu counter")
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Cc: David Laight <david.laight.linux@xxxxxxxxx>
Cc: Davidlohr Bueso <dave@xxxxxxxxxxxx>
Cc: Manfred Spraul <manfred@xxxxxxxxxxxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx>
---

 ipc/msg.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

--- a/ipc/msg.c~ipc-msg-clamp-negative-msg_info-usage-counts
+++ a/ipc/msg.c
@@ -501,10 +501,10 @@ static int msgctl_info(struct ipc_namesp
 	max_idx = ipc_get_maxidx(&msg_ids(ns));
 	up_read(&msg_ids(ns).rwsem);
 	if (cmd == MSG_INFO) {
-		msginfo->msgmap = min(percpu_counter_sum(&ns->percpu_msg_hdrs),
-				     INT_MAX);
-		msginfo->msgtql = min(percpu_counter_sum(&ns->percpu_msg_bytes),
-				     INT_MAX);
+		msginfo->msgmap = clamp(percpu_counter_sum(&ns->percpu_msg_hdrs),
+					0, INT_MAX);
+		msginfo->msgtql = clamp(percpu_counter_sum(&ns->percpu_msg_bytes),
+					0, INT_MAX);
 	} else {
 		msginfo->msgmap = MSGMAP;
 		msginfo->msgpool = MSGPOOL;
_