linux-kernel - Re: [QUESTION] inconsistent use of smp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ulg54pf2qnlzqfj247fypypzun2yvwepqrcwaqzlr6sn3ukuab@rov7btfppktc>
Date: Wed, 4 Dec 2024 09:27:22 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: Zilin Guan <zilin@....edu.cn>
Cc: dhowells@...hat.com, jlayton@...nel.org, netfs@...ts.linux.dev, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, xujianhao01@...il.com
Subject: Re: [QUESTION] inconsistent use of smp_mb()

On Wed, Dec 04, 2024 at 06:48:18AM +0000, Zilin Guan wrote:
> Hello,
> 
> I have a question regarding the use of smp_rmb() to enforce 
> memory ordering in two related functions.
> 
> In the function netfs_unbuffered_write_iter_locked() from the file 
> fs/netfs/direct_write.c, smp_rmb() is explicitly used after the 
> wait_on_bit() call to ensure that the error and transferred fields are 
> read in the correct order following the NETFS_RREQ_IN_PROGRESS flag:
> 
> 105	wait_on_bit(&wreq->flags, NETFS_RREQ_IN_PROGRESS,
> 106		    TASK_UNINTERRUPTIBLE);
> 107	smp_rmb(); /* Read error/transferred after RIP flag */
> 108	ret = wreq->error;
> 109	if (ret == 0) {
> 110		ret = wreq->transferred;
> 111		iocb->ki_pos += ret;
> 112	}
> 
> However, in the function netfs_end_writethrough() from the file 
> fs/netfs/write_issue.c, there is no such use of smp_rmb() after 
> the corresponding wait_on_bit() call, despite accessing the same filed 
> of wreq->error and relying on the same NETFS_RREQ_IN_PROGRESS flag:
> 
> 681	wait_on_bit(&wreq->flags, NETFS_RREQ_IN_PROGRESS, 
> 		    TASK_UNINTERRUPTIBLE);
> 682	ret = wreq->error;
> 
> My question is why does the first function require a CPU memory barrier 
> smp_rmb() to enforce ordering, whereas the second function does not?

The fence is redundant.

Per the comment in wait_on_bit:
 * Returned value will be zero if the bit was cleared in which case the
 * call has ACQUIRE semantics, or %-EINTR if the process received a
 * signal and the mode permitted wake up on that signal.

Since both sites pass TASK_UNINTERRUPTIBLE this will only ever return
after the bit is sorted out, already providing the needed fence.