linux-kernel - Re: [PATCH v2 1/1] nvme: Convert tag_list mutex to rwsemaphore to avoid deadlock

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: <20251118020731.GB2197103-mkhalfella@purestorage.com>
Date: Mon, 17 Nov 2025 18:07:31 -0800
From: Mohamed Khalfella <mkhalfella@...estorage.com>
To: Hillf Danton <hdanton@...a.com>
Cc: Jens Axboe <axboe@...nel.dk>, Ming Lei <ming.lei@...hat.com>,
	Waiman Long <llong@...hat.com>, linux-nvme@...ts.infradead.org,
	linux-block@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/1] nvme: Convert tag_list mutex to rwsemaphore to
 avoid deadlock

On Tue 2025-11-18 09:34:41 +0800, Hillf Danton wrote:
> On Mon, 17 Nov 2025 12:23:53 -0800 Mohamed Khalfella wrote:
> >  static void blk_mq_del_queue_tag_set(struct request_queue *q)
> >  {
> >  	struct blk_mq_tag_set *set = q->tag_set;
> > +	struct request_queue *firstq;
> > +	unsigned int memflags;
> >  
> > -	mutex_lock(&set->tag_list_lock);
> > +	down_write(&set->tag_list_rwsem);
> >  	list_del(&q->tag_set_list);
> > -	if (list_is_singular(&set->tag_list)) {
> > -		/* just transitioned to unshared */
> > -		set->flags &= ~BLK_MQ_F_TAG_QUEUE_SHARED;
> > -		/* update existing queue */
> > -		blk_mq_update_tag_set_shared(set, false);
> > +	if (!list_is_singular(&set->tag_list)) {
> > +		up_write(&set->tag_list_rwsem);
> > +		goto out;
> >  	}
> > -	mutex_unlock(&set->tag_list_lock);
> > +
> > +	/*
> > +	 * Transitioning the remaining firstq to unshared.
> > +	 * Also, downgrade the semaphore to avoid deadlock
> > +	 * with blk_mq_quiesce_tagset() while waiting for
> > +	 * firstq to be frozen.
> > +	 */
> > +	set->flags &= ~BLK_MQ_F_TAG_QUEUE_SHARED;
> > +	downgrade_write(&set->tag_list_rwsem);
> 
> If the first lock waiter is for write, it could ruin your downgrade trick.

How is that possible? If the first waiter or the only waiter is for
write then they should not take the semaphore because it has not been
fully released yet, right?