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 BUGFIX V2] block, bfq: fix use after free in
 bfq_bfqq_expire
To:     Paolo Valente <paolo.valente@linaro.org>
Cc:     linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        ulf.hansson@linaro.org, linus.walleij@linaro.org,
        broonie@kernel.org, bfq-iosched@googlegroups.com,
        oleksandr@natalenko.name, Dmitrii Tcvetkov <demfloro@demfloro.ru>,
        Douglas Anderson <dianders@chromium.org>
References: <20190410083833.14462-1-paolo.valente@linaro.org>
From:   Jens Axboe <axboe@kernel.dk>
Message-ID: <94d03c91-2e4b-6f42-7c99-09efe964cc8a@kernel.dk>
Date:   Wed, 10 Apr 2019 07:55:04 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <20190410083833.14462-1-paolo.valente@linaro.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 4/10/19 2:38 AM, Paolo Valente wrote:
> The function bfq_bfqq_expire() invokes the function
> __bfq_bfqq_expire(), and the latter may free the in-service bfq-queue.
> If this happens, then no other instruction of bfq_bfqq_expire() must
> be executed, or a use-after-free will occur.
> 
> Basing on the assumption that __bfq_bfqq_expire() invokes
> bfq_put_queue() on the in-service bfq-queue exactly once, the queue is
> assumed to be freed if its refcounter is equal to one right before
> invoking __bfq_bfqq_expire().
> 
> But, since commit 9dee8b3b057e ("block, bfq: fix queue removal from
> weights tree") this assumption is false. __bfq_bfqq_expire() may also
> invoke bfq_weights_tree_remove() and, since commit 9dee8b3b057e
> ("block, bfq: fix queue removal from weights tree"), also
> the latter function may invoke bfq_put_queue(). So __bfq_bfqq_expire()
> may invoke bfq_put_queue() twice, and this is the actual case where
> the in-service queue may happen to be freed.
> 
> To address this issue, this commit moves the check on the refcounter
> of the queue right around the last bfq_put_queue() that may be invoked
> on the queue.

Applied, thanks.

-- 
Jens Axboe