linux-kernel - Re: [PATCH 0/5]stop normal completion path entering a timeout req

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: <20180620181601.GA24145@localhost.localdomain>
Date:   Wed, 20 Jun 2018 12:16:01 -0600
From:   Keith Busch <keith.busch@...el.com>
To:     Jianchao Wang <jianchao.w.wang@...cle.com>
Cc:     axboe@...nel.dk, hch@....de, martin.petersen@...cle.com,
        josef@...icpanda.com, ulf.hansson@...aro.org,
        linux-block@...r.kernel.org, linux-scsi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/5]stop normal completion path entering a timeout req

On Wed, Jun 20, 2018 at 09:22:39PM +0800, Jianchao Wang wrote:
> Dear all
> 
> scsi timeout and error handler are based on an assumption that normal
> completion mustn't do anything on an timeout request. After 12f5b931
> (blk-mq: Remove generation seqeunce), we lost this. __blk_mq_complete
> request could ensure a request won't be completed twice, but it can
> still complete a timeout request.
> scsi (even other drivers) have been working on this assumption for many
> years, it is dangerous to discard it suddenly. This patch set is to regain this.

I certainly don't want to harm any drivers. Could you possibly explain
what about removing silent execptions from the completion handler and
letting drivers control the destiny of requests they own is "dangerous"?

A initial look at your proposal looks pretty harmful to me. A driver may
return BLK_EH_RESET_TIMER, then call blk_mq_complete_req from another
thread, and your patch will simply lose that request and escalate error
recovery. That seems exactly what you shouldn't want to happen.