linux-kernel - Re: [PATCH] btrfs: fix waitqueue_active without memory barrier in btrfs

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]

Date:	Fri, 9 Oct 2015 04:50:35 +0000
From:	Kosuke Tatsukawa <tatsu@...jp.nec.com>
To:	Josef Bacik <jbacik@...com>
CC:	Chris Mason <clm@...com>, David Sterba <dsterba@...e.com>,
	"linux-btrfs@...r.kernel.org" <linux-btrfs@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] btrfs: fix waitqueue_active without memory barrier in
 btrfs 

Josef Bacik wrote:
> On 10/08/2015 05:35 PM, Kosuke Tatsukawa wrote:
>> btrfs_bio_counter_sub() seems to be missing a memory barrier which might
>> cause the waker to not notice the waiter and miss sending a wake_up as
>> in the following figure.
>> 
>> 	btrfs_bio_counter_sub			btrfs_rm_dev_replace_blocked
>> ------------------------------------------------------------------------
>> if (waitqueue_active(&fs_info->replace_wait))
>> /* The CPU might reorder the test for
>>     the waitqueue up here, before
>>     prior writes complete */
>> 					/* wait_event */
>> 					 /* __wait_event */
>> 					  /* ___wait_event */
>> 					  long __int = prepare_to_wait_event(&wq,
>> 					    &__wait, state);
>> 					  if (!percpu_counter_sum(&fs_info->bio_counter))
>> percpu_counter_sub(&fs_info->bio_counter,
>>    amount);
>> 					  schedule()
>
> percpu_counter_sub can't be reordered, in its most basic form it does
> preempt_disable/enable which in its most basic form does barrier().  Thanks,

It's not the compiler, but the CPU that is doing the reordering.

The CPU can delay the write of the counter, so that the following read
of &fs_info->replace_wait is completed first.  Hence a memory barrier is
required, and not just a barrier.
---
Kosuke TATSUKAWA  | 3rd IT Platform Department
                  | IT Platform Division, NEC Corporation
                  | tatsu@...jp.nec.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/