Re: [PATCH net] af_unix: order MSG_PEEK references against garbage collection

[Kuniyuki Iwashima <kuniyu@google.com>]

On Thu, 22 Oct 2026 22:05:36 +0800, Yingjie Wang wrote:
> Commit e5b31d988a41 ("af_unix: Give up GC if MSG_PEEK intervened.")
> made unix_peek_fpl() invalidate unix_peek_seq when gc_in_progress is
> set.
> 
> scm_fp_dup() increments the received files' reference counts before
> unix_peek_fpl() checks that flag, while unix_gc() sets the flag before
> reading the same reference counts. The reference-count operations and
> flag accesses are relaxed. spin_lock() provides acquire ordering, but
> does not order the earlier flag store against the later file_count()
> loads.
> 
> On weakly ordered machines, GC can therefore miss the reference acquired
> by MSG_PEEK while MSG_PEEK misses gc_in_progress. unix_peek_seq then
> remains unchanged, allowing GC to classify an SCC as dead and purge a
> live receive queue.
> 
> Add smp_mb__after_atomic() after scm_fp_dup()'s reference increments and
> smp_mb__after_spinlock() before GC samples the reference counts. This
> ensures that either GC observes the new reference or MSG_PEEK observes
> the active GC and invalidates the seqcount.
> 
> LKMM admits the stale/stale outcome without these barriers and forbids it
> with them. An explicit AArch64 model gives the same result. The original
> workqueue layout in that commit also lacked the PEEK-side ordering, so the
> incomplete handshake dates to e5b31d988a41.
> 
> Fixes: e5b31d988a41 ("af_unix: Give up GC if MSG_PEEK intervened.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yingjie Wang <1075151112@qq.com>
> ---
>  net/unix/garbage.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/net/unix/garbage.c b/net/unix/garbage.c
> index da774f56ca648..c3ed00f52b04e 100644
> --- a/net/unix/garbage.c
> +++ b/net/unix/garbage.c
> @@ -329,6 +329,14 @@ void unix_peek_fpl(struct scm_fp_list *fpl)
>  	if (!fpl || !fpl->count_unix)
>  		return;
>  
> +	/*
> +	 * Pair with smp_mb__after_spinlock() in unix_gc(). get_file() in
> +	 * scm_fp_dup() increments an atomic reference count, so this ensures
> +	 * that either GC sees the new references or this thread sees
> +	 * gc_in_progress and invalidates unix_peek_seq.
> +	 */
> +	smp_mb__after_atomic();
> +

Thanks for the patch, and sorry for the late reply.

Actually there is another thread going on for this issue and
I suggested another aproach, so ...

pw-bot: cr



>  	if (!READ_ONCE(gc_in_progress))
>  		return;
>  
> @@ -614,6 +622,12 @@ static void unix_gc(struct work_struct *work)
>  	WRITE_ONCE(gc_in_progress, true);
>  
>  	spin_lock(&unix_gc_lock);
> +	/*
> +	 * Pair with smp_mb__after_atomic() in unix_peek_fpl() to ensure that
> +	 * either the file-count reads below see MSG_PEEK's increments or
> +	 * MSG_PEEK sees gc_in_progress.
> +	 */
> +	smp_mb__after_spinlock();
>  
>  	if (unix_graph_state == UNIX_GRAPH_NOT_CYCLIC) {
>  		spin_unlock(&unix_gc_lock);
> 
> base-commit: f4377bf85887f57b59abb5479f7660a81ffe4449
> -- 
> 2.43.0
>