Re: [PATCH v2] ipc: only destroy orphaned shm segments on sysctl write

[Davidlohr Bueso]

On Thu, 25 Jun 2026, Andrew Morton wrote:

> On Mon, 15 Jun 2026 10:27:41 +0800 Jianlin Shi <shijianlin11@xxxxxxxxxxx> wrote:
>
> > proc_ipc_dointvec_minmax_orphans() currently calls
> > shm_destroy_orphaned() whenever shm_rmid_forced is set, including on
> > sysctl reads. Reading /proc/sys/kernel/shm_rmid_forced should not take
> > shm_ids rwsem for write and walk all segments.
> >
> > Only run the cleanup when the sysctl is written and the forced RMID
> > policy is enabled.
> >
> > When shm_rmid_forced=1, monitoring tools that read
> > /proc/sys/kernel/shm_rmid_forced trigger the cleanup on every read.
> >
> > ...
> >
> > --- a/ipc/ipc_sysctl.c
> > +++ b/ipc/ipc_sysctl.c
> > @@ -28,7 +28,7 @@ static int proc_ipc_dointvec_minmax_orphans(const struct ctl_table *table, int w
> >
> > 	if (err < 0)
> > 		return err;
> > -	if (ns->shm_rmid_forced)
> > +	if (write && ns->shm_rmid_forced)
> > 		shm_destroy_orphaned(ns);
> > 	return err;
> >  }
>
> Seems reasonable, I guess.  Maybe.  It's not backward compatible.
>
> Nothing in the shm_rmid_forced documentation
> (Documentation/admin-guide/sysctl/kernel.rst) indicates that a read has
> any such side-effect.
>
> But there's a risk that users have discovered that a read has this
> effect, so they're using reads because that's a lot less hassle than
> figuring out what value to write.
>
> Davidlohr, Eric: any thoughts on this?

The only way I can justify this is that the doc is explicit about the
write, so users abusing the read can't really complain... and this is
also the more intuitive user expectation, and prevents unprivileged
users from doing destructive actions.

But also, afaict, for multi namespace use cases, this read effect has
less impact as of 85b6d24646e4 ("shm: extend forced shm destroy to support
objects from several IPC nses"). After that commit the reaping is done
per-segment - exit_shm() uses the segment's namespace instead of the
exiting task's - so the segment is destroyed by exit_shm() if the
creator dies while it is unattached, or by shm_close() on the last
detach, leaving a later sysctl read with nothing to find - albeit
still grabbing locks.

Acked-by: Davidlohr Bueso <dave@xxxxxxxxxxxx>

(but I'm not opposed to dropping this patch and leaving this as is if
this is not a real problem/use case)

> btw, the doc says "if you change this from 0 to 1, already created
> segments without users and with a dead originative process will be
> destroyed" but the implementation will do the destruction for any write
> of a non-zero value.  It should read "if a non-zero value is written
> already created segments without users and with a dead originative
> process will be destroyed".  Minor detail.

Yes that non-zero rewording would match the code, but the doc's
explicit 0->1 does suggest doing the reap only when first enabling
it, instead of on demand if already enabled. But ultimately I agree
with this "non-zero" in the doc.

Thanks,
Davidlohr