Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH -next] ipc/sem: prevent queue.status tearing in semop
To:     Davidlohr Bueso <dave@stgolabs.net>, akpm@linux-foundation.org
Cc:     linux-kernel@vger.kernel.org, Davidlohr Bueso <dbueso@suse.de>
References: <20180717052654.676-1-dave@stgolabs.net>
From:   Manfred Spraul <manfred@colorfullife.com>
Message-ID: <7909e12b-6dd7-e28a-010c-003545a8e4b5@colorfullife.com>
Date:   Wed, 18 Jul 2018 05:55:11 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.8.0
MIME-Version: 1.0
In-Reply-To: <20180717052654.676-1-dave@stgolabs.net>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hello Davidlohr,

On 07/17/2018 07:26 AM, Davidlohr Bueso wrote:
> In order for load/store tearing to work, _all_ accesses to
> the variable in question need to be done around READ and
> WRITE_ONCE() macros. Ensure everyone does so for q->status
> variable for semtimedop().
What is the background of the above rule?

sma->use_global_lock is sometimes used with smp_load_acquire(), 
sometimes without.
So far, I assumed that this is safe.

The same applies for nf_conntrack_locks_all, in nf_conntrack_all_lock()
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
> ---
>   ipc/sem.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ipc/sem.c b/ipc/sem.c
> index 6cbbf34a44ac..ccab4e51d351 100644
> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -2125,7 +2125,7 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops,
>   	}
>   
>   	do {
> -		queue.status = -EINTR;
> +		WRITE_ONCE(queue.status, -EINTR);
>   		queue.sleeper = current;
>   
>   		__set_current_state(TASK_INTERRUPTIBLE);