linux-kernel - Re: [PATCH -next] ipc/sem: prevent queue.status tearing in semop

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: <7909e12b-6dd7-e28a-010c-003545a8e4b5@colorfullife.com>
Date:   Wed, 18 Jul 2018 05:55:11 +0200
From:   Manfred Spraul <manfred@...orfullife.com>
To:     Davidlohr Bueso <dave@...olabs.net>, akpm@...ux-foundation.org
Cc:     linux-kernel@...r.kernel.org, Davidlohr Bueso <dbueso@...e.de>
Subject: Re: [PATCH -next] ipc/sem: prevent queue.status tearing in semop

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@...e.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);