linux-kernel - Re: [PATCH v3 2/3] scsi: ufs: Fix a race condition btw task management request send and compl

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: <73362ca9-93be-c38f-a881-4b7cf690fbc1@acm.org>
Date:   Thu, 28 Jan 2021 19:20:07 -0800
From:   Bart Van Assche <bvanassche@....org>
To:     Can Guo <cang@...eaurora.org>, jaegeuk@...nel.org,
        asutoshd@...eaurora.org, nguyenb@...eaurora.org,
        hongwus@...eaurora.org, linux-scsi@...r.kernel.org,
        kernel-team@...roid.com
Cc:     Alim Akhtar <alim.akhtar@...sung.com>,
        Avri Altman <avri.altman@....com>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Stanley Chu <stanley.chu@...iatek.com>,
        Bean Huo <beanhuo@...ron.com>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/3] scsi: ufs: Fix a race condition btw task
 management request send and compl

On 1/27/21 8:16 PM, Can Guo wrote:
> ufshcd_compl_tm() looks for all 0 bits in the REG_UTP_TASK_REQ_DOOR_BELL
> and call complete() for each req who has the req->end_io_data set. There
> can be a race condition btw tmc send/compl, because the req->end_io_data is
> set, in __ufshcd_issue_tm_cmd(), without host lock protection, so it is
> possible that when ufshcd_compl_tm() checks the req->end_io_data, it is set
> but the corresponding tag has not been set in REG_UTP_TASK_REQ_DOOR_BELL.
> Thus, ufshcd_tmc_handler() may wrongly complete TMRs which have not been
> sent out. Fix it by protecting req->end_io_data with host lock, and let
> ufshcd_compl_tm() only handle those tm cmds which have been completed
> instead of looking for 0 bits in the REG_UTP_TASK_REQ_DOOR_BELL.

I don't know any other block driver that needs locking to protect races
between submission and completion context. Can the block layer timeout
mechanism be used instead of the mechanism introduced by this patch,
e.g. by using blk_execute_rq_nowait() to submit requests? That would
allow to reuse the existing mechanism in the block layer core to handle
races between request completion and timeout handling.

Thanks,

Bart.