linux-kernel - Re: INFO: task hung in blk_queue

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: <6db16aa3a7c56b6dcca2d10b4e100a780c740081.camel@wdc.com>
Date:   Wed, 16 May 2018 15:37:24 +0000
From:   Bart Van Assche <Bart.VanAssche@....com>
To:     "dvyukov@...gle.com" <dvyukov@...gle.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "jthumshirn@...e.de" <jthumshirn@...e.de>,
        "alan.christopher.jenkins@...il.com" 
        <alan.christopher.jenkins@...il.com>,
        "syzbot+c4f9cebf9d651f6e54de@...kaller.appspotmail.com" 
        <syzbot+c4f9cebf9d651f6e54de@...kaller.appspotmail.com>,
        "penguin-kernel@...ove.sakura.ne.jp" 
        <penguin-kernel@...ove.sakura.ne.jp>,
        "martin.petersen@...cle.com" <martin.petersen@...cle.com>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        "dan.j.williams@...el.com" <dan.j.williams@...el.com>,
        "hch@....de" <hch@....de>,
        "oleksandr@...alenko.name" <oleksandr@...alenko.name>,
        "ming.lei@...hat.com" <ming.lei@...hat.com>,
        "martin@...htvoll.de" <martin@...htvoll.de>,
        "hare@...e.com" <hare@...e.com>,
        "syzkaller-bugs@...glegroups.com" <syzkaller-bugs@...glegroups.com>,
        "ross.zwisler@...ux.intel.com" <ross.zwisler@...ux.intel.com>,
        "keith.busch@...el.com" <keith.busch@...el.com>,
        "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>
Subject: Re: INFO: task hung in blk_queue_enter

On Wed, 2018-05-16 at 17:16 +0200, Dmitry Vyukov wrote:
> On Wed, May 16, 2018 at 4:56 PM, Bart Van Assche <Bart.VanAssche@....com> wrote:
> > On Wed, 2018-05-16 at 22:05 +0900, Tetsuo Handa wrote:
> > > diff --git a/block/blk-core.c b/block/blk-core.c
> > > index 85909b4..59e2496 100644
> > > --- a/block/blk-core.c
> > > +++ b/block/blk-core.c
> > > @@ -951,10 +951,10 @@ int blk_queue_enter(struct request_queue *q, blk_mq_req_flags_t flags)
> > >               smp_rmb();
> > > 
> > >               wait_event(q->mq_freeze_wq,
> > > -                        (atomic_read(&q->mq_freeze_depth) == 0 &&
> > > -                         (preempt || !blk_queue_preempt_only(q))) ||
> > > +                        atomic_read(&q->mq_freeze_depth) ||
> > > +                        (preempt || !blk_queue_preempt_only(q)) ||
> > >                          blk_queue_dying(q));
> > > -             if (blk_queue_dying(q))
> > > +             if (atomic_read(&q->mq_freeze_depth) || blk_queue_dying(q))
> > >                       return -ENODEV;
> > >       }
> > >  }
> > 
> > That change looks wrong to me.
> 
> Hi Bart,
> 
> Why does it look wrong to you?

Because that change conflicts with the purpose of queue freezing and also because
that change would inject I/O errors in code paths that shouldn't inject I/O errors.
Please have a look at e.g. generic_make_request(). From the start of that function:

	if (blk_queue_enter(q, flags) < 0) {
		if (!blk_queue_dying(q) && (bio->bi_opf & REQ_NOWAIT))
			bio_wouldblock_error(bio);
		else
			bio_io_error(bio);
		return ret;
	}

The above patch changes the behavior of blk_queue_enter() code from waiting while
q->mq_freeze_depth != 0 into returning -ENODEV while the request queue is frozen.
That will cause generic_make_request() to call bio_io_error(bio) while a request
queue is frozen if REQ_NOWAIT has not been set, which is the default behavior. So
any operation that freezes the queue temporarily, e.g. changing the queue depth,
concurrently with I/O processing can cause I/O to fail with -ENODEV. As you
probably know failure of write requests has very annoying consequences. It e.g.
causes filesystems to go into read-only mode. That's why I think that the above
change is completely wrong.

Bart.