linux-kernel - Re: [RESEND PATCH] nvme: Use spin_lock_irqsave() when taking the ctrl->lock

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: <20200812211959.GA1238450@dhcp-10-100-145-180.wdl.wdc.com>
Date:   Wed, 12 Aug 2020 14:19:59 -0700
From:   Keith Busch <kbusch@...nel.org>
To:     Logan Gunthorpe <logang@...tatee.com>
Cc:     linux-kernel@...r.kernel.org, linux-nvme@...ts.infradead.org,
        linux-block@...r.kernel.org, Christoph Hellwig <hch@....de>,
        Sagi Grimberg <sagi@...mberg.me>,
        Keith Busch <keith.busch@....com>, Jens Axboe <axboe@...nel.dk>
Subject: Re: [RESEND PATCH] nvme: Use spin_lock_irqsave() when taking the
 ctrl->lock

On Wed, Aug 12, 2020 at 03:01:19PM -0600, Logan Gunthorpe wrote:
> @@ -2971,15 +2971,16 @@ int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp, u8 csi,
>  static struct nvme_cel *nvme_find_cel(struct nvme_ctrl *ctrl, u8 csi)
>  {
>  	struct nvme_cel *cel, *ret = NULL;
> +	unsigned long flags;
> 
> -	spin_lock(&ctrl->lock);
> +	spin_lock_irqsave(&ctrl->lock, flags);
>  	list_for_each_entry(cel, &ctrl->cels, entry) {
>  		if (cel->csi == csi) {
>  			ret = cel;
>  			break;
>  		}
>  	}
> -	spin_unlock(&ctrl->lock);
> +	spin_unlock_irqrestore(&ctrl->lock, flags);
> 
>  	return ret;
>  }
> @@ -2988,6 +2989,7 @@ static int nvme_get_effects_log(struct nvme_ctrl *ctrl, u8 csi,
>  				struct nvme_effects_log **log)
>  {
>  	struct nvme_cel *cel = nvme_find_cel(ctrl, csi);
> +	unsigned long flags;
>  	int ret;
> 
>  	if (cel)
> @@ -3006,9 +3008,9 @@ static int nvme_get_effects_log(struct nvme_ctrl *ctrl, u8 csi,
> 
>  	cel->csi = csi;
> 
> -	spin_lock(&ctrl->lock);
> +	spin_lock_irqsave(&ctrl->lock, flags);
>  	list_add_tail(&cel->entry, &ctrl->cels);
> -	spin_unlock(&ctrl->lock);
> +	spin_unlock_irqrestore(&ctrl->lock, flags);
>  out:
>  	*log = &cel->log;
>  	return 0;
> 

Neither of these are ever called from an interrupt disabled context,
correct? If so, you can just use spin_lock_irq() without saving the
current irq state.