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

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

[Jianlin Shi]

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.

Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
 ipc/ipc_sysctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
index 71d0a4ef5..8bf0e9a5f 100644
--- 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;
 }
-- 
2.43.0

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

[Jianlin Shi]

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.

Changes in v2:
- No functional changes since v1
- Reposting after no responses for two weeks (v1 sent on 2 Jun 2026)

Link: https://lore.kernel.org/all/?q=only+destroy+orphaned+shm+segments+on+sysctl+write

Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
 ipc/ipc_sysctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
index 71d0a4ef5..8bf0e9a5f 100644
--- 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;
 }
-- 
2.43.0

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

[Andrew Morton]

On Mon, 15 Jun 2026 10:27:41 +0800 Jianlin Shi <shijianlin11@foxmail.com> 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?


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.

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

[Jianlin Shi]

On Thu, 25 Jun 2026 16:06:01 -0700 Andrew Morton wrote:
> 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?

Friendly ping.

I agree the documented behavior does not mention any read side-effect,
so matching the documentation seems correct. I am happy to follow
whatever Eric and Davidlohr prefer on the compatibility question.

Regarding the documentation wording you mentioned (0-to-1 vs any
non-zero write), I can send a follow-up doc fix if that is useful.

Thanks,
Jianlin

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@foxmail.com> 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@stgolabs.net>

(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