linux-kernel - Re: [PATCH 4/4] blk-mq: fix mq_usage_counter race when switching to percpu mode

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:	Mon, 29 Jun 2015 23:31:19 +0900
From:	Akinobu Mita <akinobu.mita@...il.com>
To:	Ming Lei <tom.leiming@...il.com>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Jens Axboe <axboe@...nel.dk>
Subject: Re: [PATCH 4/4] blk-mq: fix mq_usage_counter race when switching to
 percpu mode

2015-06-24 21:35 GMT+09:00 Ming Lei <tom.leiming@...il.com>:
> On Sun, Jun 21, 2015 at 9:52 PM, Akinobu Mita <akinobu.mita@...il.com> wrote:
>> percpu_ref_switch_to_percpu() and percpu_ref_kill() must not be
>> executed at the same time as the following scenario is possible:
>>
>> 1. q->mq_usage_counter is initialized in atomic mode.
>>    (atomic counter: 1)
>>
>> 2. After the disk registration, a process like systemd-udev starts
>>    accessing the disk, and successfully increases refcount successfully
>>    by percpu_ref_tryget_live() in blk_mq_queue_enter().
>>    (atomic counter: 2)
>>
>> 3. In the final stage of initialization, q->mq_usage_counter is being
>>    switched to percpu mode by percpu_ref_switch_to_percpu() in
>>    blk_mq_finish_init().  But if CONFIG_PREEMPT_VOLUNTARY is enabled,
>>    the process is rescheduled in the middle of switching when calling
>>    wait_event() in __percpu_ref_switch_to_percpu().
>>    (atomic counter: 2)
>
> This can only happen when freezing queue from CPU
> hotplug happens before running wait_event() from blk_mq_finish_init().
>
> So looks like we still may avoid the race by moving adding queue
> into all_q_list to blk_mq_register_disk(), but the mapping need to
> update at that time.

As you suggested, we can avoid the problems described in patch 1/4,
2/4, and 4/4 by adding to all_q_list in blk_mq_register_disk() and
removing from all_q_list in blk_mq_unregister_disk().

On the other hand, that change breaks the patch 3/4.  Because it
allows the requests to be inserted before adding the queue to
all_q_list.  As we have disscussed about patch 3/4, there is a problem
if the request is inserted on the just onlined CPU before establishing
new mapping.
--
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/