linux-kernel - Re: false positive lockdep splat with loop device

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] [day] [month] [year] [list]

Date:   Tue, 10 Oct 2017 11:43:18 +0200
From:   Ilya Dryomov <idryomov@...il.com>
To:     Christoph Hellwig <hch@...radead.org>
Cc:     Amir Goldstein <amir73il@...il.com>,
        Dave Chinner <david@...morbit.com>,
        "Darrick J. Wong" <darrick.wong@...cle.com>,
        xfs <linux-xfs@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Byungchul Park <byungchul.park@....com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        linux-block <linux-block@...r.kernel.org>
Subject: Re: false positive lockdep splat with loop device

On Tue, Oct 10, 2017 at 11:16 AM, Ilya Dryomov <idryomov@...il.com> wrote:
> On Thu, Oct 5, 2017 at 6:33 PM, Christoph Hellwig <hch@...radead.org> wrote:
>> Does the patch below fix the warning for you?
>>
>> --
>> From 28aae7104425433d39e6142adcd5b88dc5b0ad5f Mon Sep 17 00:00:00 2001
>> From: Christoph Hellwig <hch@....de>
>> Date: Thu, 5 Oct 2017 18:31:02 +0200
>> Subject: block: use DECLARE_COMPLETION_ONSTACK in submit_bio_wait
>>
>> This way we get our lockdep annotations right in case multiple layers
>> in the stack use submit_bio_wait.
>>
>> It also happens to simplify the code by getting rid of the submit_bio_ret
>> structure.
>>
>> Signed-off-by: Christoph Hellwig <hch@....de>
>> ---
>>  block/bio.c | 19 +++++--------------
>>  1 file changed, 5 insertions(+), 14 deletions(-)
>>
>> diff --git a/block/bio.c b/block/bio.c
>> index 8338304ea256..4e18e959fc0a 100644
>> --- a/block/bio.c
>> +++ b/block/bio.c
>> @@ -917,17 +917,9 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
>>  }
>>  EXPORT_SYMBOL_GPL(bio_iov_iter_get_pages);
>>
>> -struct submit_bio_ret {
>> -       struct completion event;
>> -       int error;
>> -};
>> -
>>  static void submit_bio_wait_endio(struct bio *bio)
>>  {
>> -       struct submit_bio_ret *ret = bio->bi_private;
>> -
>> -       ret->error = blk_status_to_errno(bio->bi_status);
>> -       complete(&ret->event);
>> +       complete(bio->bi_private);
>>  }
>>
>>  /**
>> @@ -943,16 +935,15 @@ static void submit_bio_wait_endio(struct bio *bio)
>>   */
>>  int submit_bio_wait(struct bio *bio)
>>  {
>> -       struct submit_bio_ret ret;
>> +       DECLARE_COMPLETION_ONSTACK(done);
>>
>> -       init_completion(&ret.event);
>> -       bio->bi_private = &ret;
>> +       bio->bi_private = &done;
>>         bio->bi_end_io = submit_bio_wait_endio;
>>         bio->bi_opf |= REQ_SYNC;
>>         submit_bio(bio);
>> -       wait_for_completion_io(&ret.event);
>> +       wait_for_completion_io(&done);
>>
>> -       return ret.error;
>> +       return blk_status_to_errno(bio->bi_status);
>>  }
>>  EXPORT_SYMBOL(submit_bio_wait);
>
> No, it doesn't -- the splat is a little bit more complicated, but
> fundamentally the same thing.

Easily triggered with generic/361 too, BTW.

Thanks,

                Ilya