linux-kernel - Re: [PATCH v9 2/2] blk-mq: Rework blk-mq timeout handling again

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: <20180516162432.GA4398@lst.de>
Date:   Wed, 16 May 2018 18:24:32 +0200
From:   "hch@....de" <hch@....de>
To:     Bart Van Assche <Bart.VanAssche@....com>
Cc:     "hch@....de" <hch@....de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "israelr@...lanox.com" <israelr@...lanox.com>,
        "sagi@...mberg.me" <sagi@...mberg.me>,
        "sebott@...ux.ibm.com" <sebott@...ux.ibm.com>,
        "ming.lei@...hat.com" <ming.lei@...hat.com>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        "jianchao.w.wang@...cle.com" <jianchao.w.wang@...cle.com>,
        "maxg@...lanox.com" <maxg@...lanox.com>,
        "tj@...nel.org" <tj@...nel.org>
Subject: Re: [PATCH v9 2/2] blk-mq: Rework blk-mq timeout handling again

On Wed, May 16, 2018 at 04:17:42PM +0000, Bart Van Assche wrote:
> There is another reason the deadline is included in the atomic operation,
> namely to handle races between the BLK_EH_RESET_TIMER case in blk_mq_rq_timed_out()
> and blk_mq_complete_request(). I don't think that race is addressed properly by
> your patch. I will see what I can do to address that race without using 64-bit
> atomic operations.

I might be missing something here, so please help me understand
what is missing.

If we restart the timer in blk_mq_rq_timed_out we also bump the
generation at the same time as we reset the deadline in your old
patch.  With this patch we only bump the generation, but that should
be enough to address the rest, or not?