linux-kernel - Re: [REGRESSION] "split bio_kmalloc from bio_alloc_bioset" causing crash shortly after bootup

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: <BYAPR04MB496545CBAACF978B9E11B77B86809@BYAPR04MB4965.namprd04.prod.outlook.com>
Date:   Tue, 23 Feb 2021 04:58:38 +0000
From:   Chaitanya Kulkarni <Chaitanya.Kulkarni@....com>
To:     John Stultz <john.stultz@...aro.org>
CC:     David Anderson <dvander@...gle.com>,
        Christoph Hellwig <hch@....de>,
        Johannes Thumshirn <Johannes.Thumshirn@....com>,
        Damien Le Moal <Damien.LeMoal@....com>,
        Jens Axboe <axboe@...nel.dk>,
        Alistair Delva <adelva@...gle.com>,
        Todd Kjos <tkjos@...gle.com>,
        Amit Pundir <amit.pundir@...aro.org>,
        YongQin Liu <yongqin.liu@...aro.org>,
        lkml <linux-kernel@...r.kernel.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>
Subject: Re: [REGRESSION] "split bio_kmalloc from bio_alloc_bioset" causing
 crash shortly after bootup

On 2/22/21 20:22, John Stultz wrote:
> On Mon, Feb 22, 2021 at 7:39 PM Chaitanya Kulkarni
> <Chaitanya.Kulkarni@....com> wrote:
>> On 2/22/21 19:07, John Stultz wrote:
>>> [   34.784901] ueventd: LoadWithAliases was unable to load platform:regulatory
>>> [   34.785313]  bio_alloc_bioset+0x14/0x230
>>> [   34.796189]  bio_clone_fast+0x28/0x80
>>> [   34.799848]  bio_split+0x50/0xd0
>>> [   34.803072]  blk_crypto_fallback_encrypt_bio+0x2ec/0x5e8
>>> [   34.808384]  blk_crypto_fallback_bio_prep+0xfc/0x140
>>> [   34.813345]  __blk_crypto_bio_prep+0x13c/0x150
>>> [   34.817784]  submit_bio_noacct+0x3c0/0x548
>>> [   34.821880]  submit_bio+0x48/0x200
>>> [   34.825278]  ext4_io_submit+0x50/0x68
>>> [   34.828939]  ext4_writepages+0x558/0xca8
>>> [   34.832860]  do_writepages+0x58/0x108
>>> [   34.836522]  __writeback_single_inode+0x44/0x510
>>> [   34.841137]  writeback_sb_inodes+0x1e0/0x4a8
>>> [   34.845404]  __writeback_inodes_wb+0x78/0xe8
>>> [   34.849670]  wb_writeback+0x274/0x3e8
>>> [   34.853328]  wb_workfn+0x308/0x5f0
>>> [   34.856726]  process_one_work+0x1ec/0x4d0
>>> [   34.860734]  worker_thread+0x44/0x478
>>> [   34.864392]  kthread+0x140/0x150
>>> [   34.867618]  ret_from_fork+0x10/0x30
>>> [   34.871197] Code: a9ba7bfd 910003fd f9000bf3 7900bfa1 (f9403441)
>>> [   34.877289] ---[ end trace e6c2a3ab108278f0 ]---
>>> [   34.893636] Kernel panic - not syncing: Oops: Fatal exception
>>>
>> If you have time then until you get the reply from others, can you try
>> following patch ?
>>
>> diff --git a/block/bio.c b/block/bio.c
>> index a1c4d2900c7a..9976400ec66a 100644
>> --- a/block/bio.c
>> +++ b/block/bio.c
>> @@ -663,7 +663,10 @@ struct bio *bio_clone_fast(struct bio *bio, gfp_t
>> gfp_mask, struct bio_set *bs)
>>  {
>>         struct bio *b;
>>
>> -       b = bio_alloc_bioset(gfp_mask, 0, bs);
>> +       if (bs)
>> +               b = bio_alloc_bioset(gfp_mask, 0, bs);
>> +       else
>> +               b = bio_kmalloc(gfp_mask, 0);
>>         if (!b)
>>                 return NULL;
>>
>> P.S.This is purely based on the code inspection and it may not solve your
>> issue. Proceed with the caution as it may *break* your system.
> So with an initial quick test, this patch (along with the follow-on
> one you sent) seems to avoid the issue.
Thanks for bisecting, the patch you are referring to is with the similar
fix for bio bounce ? 
> I'm wondering if given there are multiple call sites, that in
> bio_alloc_bioset() would something like the following make more sense?
> (apologies, copy pasted so this is whitespace corrupted)
> thanks
> -john
I've asked Christoph about how we can go about this issue, based on his
reply I can send a patch.

I think having kmalloc is what we want to avoid in the fast path, with
that in
mind in my opinion we need to fix the call sites with kmalloc call if number
is small,  which I think it is. Let's wait.

Meanwhile it will be great if you can share the result of the thorough
testing.

> diff --git a/block/bio.c b/block/bio.c
> index a1c4d2900c7a..391d5cde79fc 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -402,6 +402,9 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask,
> unsigned short nr_iovecs,
>         struct bio *bio;
>         void *p;
>
> +       if(!bs)
> +               return bio_kmalloc(gfp_mask, 0);
it may not work since nr_iovecs needs to be passed
> +
>         /* should not use nobvec bioset for nr_iovecs > 0 */
>         if (WARN_ON_ONCE(!mempool_initialized(&bs->bvec_pool) && nr_iovecs > 0))
>                 return NULL;
>