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

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

[Yingjie Wang]

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();
+
 	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

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

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.