From nobody Mon Sep 28 08:51:25 2026 Received: from www262.sakura.ne.jp (www262.sakura.ne.jp [202.181.97.72]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6E96141687C for ; Mon, 24 Aug 2026 12:30:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.181.97.72 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787574611; cv=none; b=p81sApTcdAO5w02I6n0b1SfyBTghbSbqHXzl2W4064XwE9DEfQPkN2Dl/tuoQx1XnYFKT8xMCBQgmF3z53N5E7kkTKZPpQtGAvmBXhjZUnWxpMAIFE8w/oYasExIz8A+5lU7ic24KAzIXC4/vo++IjOWvZhbT6b1HhBJYvGckso= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787574611; c=relaxed/simple; bh=4kXn8S4zRUbMr7658EYnQTFGBO9erg8xf7gH9bkmBos=; h=Message-ID:Date:MIME-Version:To:From:Subject:Content-Type; b=WdZJXAdTYfcVCfHjLjOn08Fk1bI1Rbh0dAz538OEA7RfUZjE+buI8P0Q1p1clE0R1rH9Xnx2qIt6efF6huAspnjgMp67Vc/dPCn9UyPvDDtl6nyI5V+R6+kyb5YN5slYalnibpfkZyRRZvgDRYPBs9q3xfqciHLoiedbgzfLf/A= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=I-love.SAKURA.ne.jp; spf=pass smtp.mailfrom=I-love.SAKURA.ne.jp; arc=none smtp.client-ip=202.181.97.72 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=I-love.SAKURA.ne.jp Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=I-love.SAKURA.ne.jp Received: from www262.sakura.ne.jp (localhost [127.0.0.1]) by www262.sakura.ne.jp (8.15.2/8.15.2) with ESMTP id 67OCTvlq095475 for ; Mon, 24 Aug 2026 21:29:57 +0900 (JST) (envelope-from penguin-kernel@I-love.SAKURA.ne.jp) Received: from [192.168.1.6] (M106072072000.v4.enabler.ne.jp [106.72.72.0]) (authenticated bits=0) by www262.sakura.ne.jp (8.15.2/8.15.2) with ESMTPSA id 67OCTv24095472 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NO) for ; Mon, 24 Aug 2026 21:29:57 +0900 (JST) (envelope-from penguin-kernel@I-love.SAKURA.ne.jp) Message-ID: <2efb1032-34b9-4c31-b6fc-4aa9b1d60895@I-love.SAKURA.ne.jp> Date: Mon, 24 Aug 2026 21:29:53 +0900 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: LKML From: Tetsuo Handa Subject: [PATCH (resend)] loop: squashed loop module changes Content-Transfer-Encoding: quoted-printable X-Anti-Virus-Server: fsav412.rs.sakura.ne.jp X-Virus-Status: clean Content-Type: text/plain; charset="utf-8" From: Bart Van Assche This is a snapshot of https://github.com/bvanassche/linux/commits/block-loo= p/ . This snapshot includes a patch: When closing a loop device, lo_release() invokes __loop_clr_fd() when disk_openers(disk) =3D=3D 0. However, disk_openers(disk) =3D=3D 0 does not guarantee that all I/O submitted by loop_queue_rq() has completed. Asynchronous direct I/O requests submitted via io_uring or Linux AIO can remain in-flight while the device is closed, and worker threads may still be processing enqueued commands. If __loop_clr_fd() clears lo->lo_backing_file to NULL without freezing the request queue, in-flight commands may read a NULL backing file pointer or access backing structures after fput(), resulting in NULL pointer dereferences or use-after-free bugs. Fix this by freezing lo->lo_queue in __loop_clr_fd() before clearing lo->lo_backing_file and unfreezing the queue after updating the queue limits. Hold lo->lo_mutex before calling queue_limits_start_update() and blk_mq_freeze_queue() to follow the established lock ordering (&lo->lo_mutex -> &q->limits_lock -> &q->q_usage_counter) and prevent circular lock dependencies with fs_reclaim. along with other improvements. Signed-off-by: Bart Van Assche --- Sashiko, I've asked you to review this patch, with can you check whether changes in this snapshot are sufficient for fixing NULL pointer dereference bug in lo_rw_aio() ? comment, and you responded that changes in this snapshot are sufficient. But I consider that this NULL pointer dereference bug in lo_rw_aio() is more complicated than what you have found. Please re-evaluate the patch by focusing on the race window between the block layer queue freeze and the loop driver's internal workqueue. I think that your previous review missed the fact that blk_mq_freeze_queue() alone CANNOT prevent a NULL pointer dereference or Use-After-Free (UAF) on `lo->lo_backing_file`. Here is the exact execution timeline that leads to the crash: 1. A pending I/O request enters loop_queue_rq(). 2. loop_queue_rq() quickly offloads this request to the driver's internal w= orkqueue (lo->workqueue) via queue_work() and immediately returns control b= ack to the block layer. 3. Concurrently, __loop_clr_fd() executes Bart's patch and calls blk_mq_fre= eze_queue(). 4. blk_mq_freeze_queue() successfully waits until the block layer's `q_usag= e_counter` drops to 0. It thinks all I/O is done. 5. However, the previously offloaded asynchronous work items (e.g., lo_rw_a= io) are STILL sitting or executing inside `lo->workqueue`. They are invisib= le to blk_mq_freeze_queue(). 6. Bart's patch proceeds to clear and free the backing file (`lo->lo_backin= g_file =3D NULL`). 7. The lingering kworker thread inside `lo->workqueue` wakes up, tries to a= ccess `lo->lo_backing_file`, and triggers a GPF / kernel panic. To fix this remaining race window, I consider that the loop driver must: - Use synchronize_rcu() to guarantee that any inflight loop_queue_rq() inst= ances that read the old state have finished exiting. - Use drain_workqueue(lo->workqueue) to explicitly flush and wait for all a= synchronously queued driver-internal kworkers to finish execution BEFORE re= sources are cleared. Please review again, with keep in mind that blk_mq_freeze_queue() does not flush driver-specific async workqueues, making synchronize_rcu() and drain_workqueue() strictly mandatory here. drivers/block/Makefile | 2 + drivers/block/loop.c | 410 +++++++++++++++++++++++++---------------- 2 files changed, 251 insertions(+), 161 deletions(-) diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 2d8096eb8cdf..d4555949cc3c 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -6,6 +6,8 @@ # Rewritten to use lists instead of if-statements. #=20 =20 +CONTEXT_ANALYSIS_loop.o :=3D y + # needed for trace events ccflags-y +=3D -I$(src) =20 diff --git a/drivers/block/loop.c b/drivers/block/loop.c index 6f12976035b0..e14d9a776dba 100644 --- a/drivers/block/loop.c +++ b/drivers/block/loop.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -37,6 +38,17 @@ #include #include =20 +/* + * Lock order: + * 1. loop_ctl_mutex: protects loop_index_idr. + * 2. disk->open_mutex: gendisk mutex held during open, release, etc. + * 3. loop_validate_mutex: global lock for loop_validate_file() check. + * 4. lo->lo_mutex: protects loop device configuration and state changes. + * 5. q->limits_lock: serializes request queue limits changes. + * 6. q->q_usage_counter: acquired by blk_mq_freeze_queue(). + * 7. lo->lo_work_lock: spinlock protecting worker lists and tree. + */ + /* Possible states of device */ enum { Lo_unbound, @@ -52,21 +64,20 @@ struct loop_device { int lo_flags; char lo_file_name[LO_NAME_SIZE]; =20 - struct file *lo_backing_file; + struct file *lo_backing_file __guarded_by(&lo_mutex); unsigned int lo_min_dio_size; unsigned int lo_dio_mem_align; struct block_device *lo_device; =20 gfp_t old_gfp_mask; =20 - spinlock_t lo_lock; - int lo_state; + int lo_state __guarded_by(&lo_mutex); spinlock_t lo_work_lock; struct workqueue_struct *workqueue; struct work_struct rootcg_work; - struct list_head rootcg_cmd_list; - struct list_head idle_worker_list; - struct rb_root worker_tree; + struct list_head rootcg_cmd_list __guarded_by(&lo_work_lock); + struct list_head idle_worker_list __guarded_by(&lo_work_lock); + struct rb_root worker_tree __guarded_by(&lo_work_lock); struct timer_list timer; bool sysfs_inited; =20 @@ -74,6 +85,7 @@ struct loop_device { struct blk_mq_tag_set tag_set; struct gendisk *lo_disk; struct mutex lo_mutex; + struct lock_class_key lo_mutex_key; bool idr_visible; }; =20 @@ -91,33 +103,32 @@ struct loop_cmd { #define LOOP_IDLE_WORKER_TIMEOUT (60 * HZ) #define LOOP_DEFAULT_HW_Q_DEPTH 128 =20 -static DEFINE_IDR(loop_index_idr); static DEFINE_MUTEX(loop_ctl_mutex); +static __guarded_by(&loop_ctl_mutex) DEFINE_IDR(loop_index_idr); static DEFINE_MUTEX(loop_validate_mutex); =20 /** * loop_global_lock_killable() - take locks for safe loop_validate_file() = test * * @lo: struct loop_device - * @global: true if @lo is about to bind another "struct loop_device", fal= se otherwise * * Returns 0 on success, -EINTR otherwise. * - * Since loop_validate_file() traverses on other "struct loop_device" if - * is_loop_device() is true, we need a global lock for serializing concurr= ent + * Since loop_validate_file() traverses on other "struct loop_device", we = need a + * global lock for serializing concurrent * loop_configure()/loop_change_fd()/__loop_clr_fd() calls. */ -static int loop_global_lock_killable(struct loop_device *lo, bool global) +static int loop_global_lock_killable(struct loop_device *lo) + __cond_acquires(0, &loop_validate_mutex) + __cond_acquires(0, &lo->lo_mutex) { int err; =20 - if (global) { - err =3D mutex_lock_killable(&loop_validate_mutex); - if (err) - return err; - } + err =3D mutex_lock_killable(&loop_validate_mutex); + if (err) + return err; err =3D mutex_lock_killable(&lo->lo_mutex); - if (err && global) + if (err) mutex_unlock(&loop_validate_mutex); return err; } @@ -126,19 +137,20 @@ static int loop_global_lock_killable(struct loop_devi= ce *lo, bool global) * loop_global_unlock() - release locks taken by loop_global_lock_killable= () * * @lo: struct loop_device - * @global: true if @lo was about to bind another "struct loop_device", fa= lse otherwise */ -static void loop_global_unlock(struct loop_device *lo, bool global) +static void loop_global_unlock(struct loop_device *lo) + __releases(&lo->lo_mutex) + __releases(&loop_validate_mutex) { mutex_unlock(&lo->lo_mutex); - if (global) - mutex_unlock(&loop_validate_mutex); + mutex_unlock(&loop_validate_mutex); } =20 static int max_part; static int part_shift; =20 static loff_t lo_calculate_size(struct loop_device *lo, struct file *file) + __must_hold(&lo->lo_mutex) { loff_t loopsize; int ret; @@ -180,6 +192,7 @@ static loff_t lo_calculate_size(struct loop_device *lo,= struct file *file) * the backing device. */ static bool lo_can_use_dio(struct loop_device *lo) + __must_hold(&lo->lo_mutex) { if (!(lo->lo_backing_file->f_mode & FMODE_CAN_ODIRECT)) return false; @@ -199,6 +212,7 @@ static bool lo_can_use_dio(struct loop_device *lo) * not the originally passed in one. */ static inline void loop_update_dio(struct loop_device *lo) + __must_hold(&lo->lo_mutex) { lockdep_assert_held(&lo->lo_mutex); WARN_ON_ONCE(lo->lo_state =3D=3D Lo_bound && @@ -217,6 +231,7 @@ static inline void loop_update_dio(struct loop_device *= lo) * a sector_t, eg using loop_validate_size() */ static void loop_set_size(struct loop_device *lo, loff_t size) + __must_hold(&lo->lo_mutex) { if (!set_capacity_and_notify(lo->lo_disk, size)) kobject_uevent(&disk_to_dev(lo->lo_disk)->kobj, KOBJ_CHANGE); @@ -251,7 +266,7 @@ static int lo_fallocate(struct loop_device *lo, struct = request *rq, loff_t pos, * We use fallocate to manipulate the space mappings used by the image * a.k.a. discard/zerorange. */ - struct file *file =3D lo->lo_backing_file; + struct file *file =3D context_unsafe(READ_ONCE(lo->lo_backing_file)); int ret; =20 mode |=3D FALLOC_FL_KEEP_SIZE; @@ -275,7 +290,7 @@ static int lo_fallocate(struct loop_device *lo, struct = request *rq, loff_t pos, =20 static int lo_req_flush(struct loop_device *lo, struct request *rq) { - int ret =3D vfs_fsync(lo->lo_backing_file, 0); + int ret =3D vfs_fsync(context_unsafe(READ_ONCE(lo->lo_backing_file)), 0); if (unlikely(ret && ret !=3D -EINVAL)) ret =3D -EIO; =20 @@ -344,7 +359,7 @@ static int lo_rw_aio(struct loop_device *lo, struct loo= p_cmd *cmd, struct iov_iter iter; struct req_iterator rq_iter; struct request *rq =3D blk_mq_rq_from_pdu(cmd); - struct file *file =3D lo->lo_backing_file; + struct file *file =3D context_unsafe(READ_ONCE(lo->lo_backing_file)); unsigned int nr_bvec; int ret; =20 @@ -449,6 +464,7 @@ static void loop_reread_partitions(struct loop_device *= lo) } =20 static void loop_update_dio_alignment(struct loop_device *lo) + __must_hold(&lo->lo_mutex) { struct file *file =3D lo->lo_backing_file; struct block_device *sb_bdev =3D file->f_mapping->host->i_sb->s_bdev; @@ -481,39 +497,70 @@ static void loop_update_dio_alignment(struct loop_dev= ice *lo) lo->lo_dio_mem_align =3D SECTOR_SIZE - 1; } =20 -static inline int is_loop_device(struct file *file) +/* Returns the block device that underpins a file. */ +static inline struct block_device *loop_get_bdev(struct file *file) +{ + struct inode *inode =3D file->f_mapping->host; + + if (S_ISBLK(inode->i_mode)) + return I_BDEV(inode); + if (S_ISREG(inode->i_mode) && inode->i_sb) + return inode->i_sb->s_bdev; + return NULL; +} + +static inline bool is_loop_device(struct file *file) { - struct inode *i =3D file->f_mapping->host; + struct block_device *bdev =3D loop_get_bdev(file); =20 - return i && S_ISBLK(i->i_mode) && imajor(i) =3D=3D LOOP_MAJOR; + return bdev && bdev->bd_disk->major =3D=3D LOOP_MAJOR; } =20 -static int loop_validate_file(struct file *file, struct block_device *bdev) +static struct file *loop_get_backing_file(struct loop_device *lo) + __must_hold(&lo->lo_mutex) +{ + if (lo->lo_state !=3D Lo_bound) + return NULL; + return get_file(lo->lo_backing_file); +} + +/* Returns 0 if and only if @file is not backed by loop device @bdev. */ +static int loop_validate_file(struct loop_device *lo, struct file *file, + struct block_device *bdev) + __must_hold(&lo->lo_mutex) { struct inode *inode =3D file->f_mapping->host; struct file *f =3D file; =20 + if (!S_ISREG(inode->i_mode) && !S_ISBLK(inode->i_mode)) + return -EINVAL; + + get_file(f); /* Avoid recursion */ while (is_loop_device(f)) { struct loop_device *l; + struct file *prev_f =3D f; + struct block_device *f_bdev =3D loop_get_bdev(f); =20 lockdep_assert_held(&loop_validate_mutex); - if (f->f_mapping->host->i_rdev =3D=3D bdev->bd_dev) + if (f_bdev->bd_disk =3D=3D bdev->bd_disk) { + fput(f); return -EBADF; + } =20 - l =3D I_BDEV(f->f_mapping->host)->bd_disk->private_data; - if (l->lo_state !=3D Lo_bound) + l =3D f_bdev->bd_disk->private_data; + scoped_guard(mutex, &l->lo_mutex) + f =3D loop_get_backing_file(l); + fput(prev_f); + if (!f) return -EINVAL; - /* Order wrt setting lo->lo_backing_file in loop_configure(). */ - rmb(); - f =3D l->lo_backing_file; } - if (!S_ISREG(inode->i_mode) && !S_ISBLK(inode->i_mode)) - return -EINVAL; + fput(f); return 0; } =20 static void loop_assign_backing_file(struct loop_device *lo, struct file *= file) + __must_hold(&lo->lo_mutex) { lo->lo_backing_file =3D file; lo->old_gfp_mask =3D mapping_gfp_mask(file->f_mapping); @@ -535,6 +582,50 @@ static int loop_check_backing_file(struct file *file) return 0; } =20 +static int __loop_change_fd(struct loop_device *lo, struct block_device *b= dev, + struct file *file, struct file **old_file, + bool *partscan) + __must_hold(&lo->lo_mutex) +{ + unsigned int memflags; + int error; + + if (lo->lo_state !=3D Lo_bound) + return -ENXIO; + + /* the loop device has to be read-only */ + if (!(lo->lo_flags & LO_FLAGS_READ_ONLY)) + return -EINVAL; + + error =3D loop_validate_file(lo, file, bdev); + if (error) + return error; + + *old_file =3D lo->lo_backing_file; + + /* size of the new backing store needs to be the same */ + if (lo_calculate_size(lo, file) !=3D lo_calculate_size(lo, *old_file)) + return -EINVAL; + + /* + * We might switch to direct I/O mode for the loop device, write back + * all dirty data the page cache now that so that the individual I/O + * operations don't have to do that. + */ + vfs_fsync(file, 0); + + /* and ... switch */ + disk_force_media_change(lo->lo_disk); + memflags =3D blk_mq_freeze_queue(lo->lo_queue); + mapping_set_gfp_mask((*old_file)->f_mapping, lo->old_gfp_mask); + loop_assign_backing_file(lo, file); + loop_update_dio(lo); + blk_mq_unfreeze_queue(lo->lo_queue, memflags); + *partscan =3D lo->lo_flags & LO_FLAGS_PARTSCAN; + + return 0; +} + /* * loop_change_fd switched the backing store of a loopback device to * a new file. This is useful for operating system installers to free up @@ -566,46 +657,21 @@ static int loop_change_fd(struct loop_device *lo, str= uct block_device *bdev, dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 1); =20 is_loop =3D is_loop_device(file); - error =3D loop_global_lock_killable(lo, is_loop); + if (is_loop) { + error =3D loop_global_lock_killable(lo); + if (error) + goto out_putf; + error =3D __loop_change_fd(lo, bdev, file, &old_file, &partscan); + loop_global_unlock(lo); + } else { + error =3D mutex_lock_killable(&lo->lo_mutex); + if (error) + goto out_putf; + error =3D __loop_change_fd(lo, bdev, file, &old_file, &partscan); + mutex_unlock(&lo->lo_mutex); + } if (error) goto out_putf; - error =3D -ENXIO; - if (lo->lo_state !=3D Lo_bound) - goto out_err; - - /* the loop device has to be read-only */ - error =3D -EINVAL; - if (!(lo->lo_flags & LO_FLAGS_READ_ONLY)) - goto out_err; - - error =3D loop_validate_file(file, bdev); - if (error) - goto out_err; - - old_file =3D lo->lo_backing_file; - - error =3D -EINVAL; - - /* size of the new backing store needs to be the same */ - if (lo_calculate_size(lo, file) !=3D lo_calculate_size(lo, old_file)) - goto out_err; - - /* - * We might switch to direct I/O mode for the loop device, write back - * all dirty data the page cache now that so that the individual I/O - * operations don't have to do that. - */ - vfs_fsync(file, 0); - - /* and ... switch */ - disk_force_media_change(lo->lo_disk); - memflags =3D blk_mq_freeze_queue(lo->lo_queue); - mapping_set_gfp_mask(old_file->f_mapping, lo->old_gfp_mask); - loop_assign_backing_file(lo, file); - loop_update_dio(lo); - blk_mq_unfreeze_queue(lo->lo_queue, memflags); - partscan =3D lo->lo_flags & LO_FLAGS_PARTSCAN; - loop_global_unlock(lo, is_loop); =20 /* * Flush loop_validate_file() before fput(), for l->lo_backing_file @@ -615,12 +681,16 @@ static int loop_change_fd(struct loop_device *lo, str= uct block_device *bdev, mutex_lock(&loop_validate_mutex); mutex_unlock(&loop_validate_mutex); } + /* - * We must drop file reference outside of lo_mutex as dropping - * the file ref can take open_mutex which creates circular locking - * dependency. + * Freeze and unfreeze the request queue to wait until old_file is + * no longer in use by the I/O path. */ + memflags =3D blk_mq_freeze_queue(lo->lo_queue); + blk_mq_unfreeze_queue(lo->lo_queue, memflags); + fput(old_file); + dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 0); if (partscan) loop_reread_partitions(lo); @@ -630,8 +700,6 @@ static int loop_change_fd(struct loop_device *lo, struc= t block_device *bdev, kobject_uevent(&disk_to_dev(lo->lo_disk)->kobj, KOBJ_CHANGE); return error; =20 -out_err: - loop_global_unlock(lo, is_loop); out_putf: fput(file); dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 0); @@ -664,10 +732,9 @@ static ssize_t loop_attr_backing_file_show(struct loop= _device *lo, char *buf) ssize_t ret; char *p =3D NULL; =20 - spin_lock_irq(&lo->lo_lock); - if (lo->lo_backing_file) - p =3D file_path(lo->lo_backing_file, buf, PAGE_SIZE - 1); - spin_unlock_irq(&lo->lo_lock); + scoped_guard(mutex, &lo->lo_mutex) + if (lo->lo_backing_file) + p =3D file_path(lo->lo_backing_file, buf, PAGE_SIZE - 1); =20 if (IS_ERR_OR_NULL(p)) ret =3D PTR_ERR(p); @@ -749,6 +816,7 @@ static void loop_sysfs_exit(struct loop_device *lo) =20 static void loop_get_discard_config(struct loop_device *lo, u32 *granularity, u32 *max_discard_sectors) + __must_hold(&lo->lo_mutex) { struct file *file =3D lo->lo_backing_file; struct inode *inode =3D file->f_mapping->host; @@ -915,6 +983,7 @@ static void loop_free_idle_workers_timer(struct timer_l= ist *timer) static int loop_set_status_from_info(struct loop_device *lo, const struct loop_info64 *info) + __must_hold(&lo->lo_mutex) { if ((unsigned int) info->lo_encrypt_key_size > LO_KEY_SIZE) return -EINVAL; @@ -953,6 +1022,7 @@ static unsigned int loop_default_blocksize(struct loop= _device *lo) } =20 static void loop_set_dma_limit(struct loop_device *lo, struct queue_limits= *lim) + __must_hold(&lo->lo_mutex) { /* * Direct I/O forwards the user pages to the backing file unchanged, so @@ -967,6 +1037,7 @@ static void loop_set_dma_limit(struct loop_device *lo,= struct queue_limits *lim) =20 static void loop_update_limits(struct loop_device *lo, struct queue_limits= *lim, unsigned int bsize) + __must_hold(&lo->lo_mutex) { struct file *file =3D lo->lo_backing_file; struct inode *inode =3D file->f_mapping->host; @@ -1000,61 +1071,29 @@ static void loop_update_limits(struct loop_device *= lo, struct queue_limits *lim, lim->discard_granularity =3D 0; } =20 -static int loop_configure(struct loop_device *lo, blk_mode_t mode, - struct block_device *bdev, - const struct loop_config *config) +static int __loop_configure(struct loop_device *lo, blk_mode_t mode, + struct block_device *bdev, + const struct loop_config *config, struct file *file, + bool *partscan) + __must_hold(&lo->lo_mutex) { - struct file *file =3D fget(config->fd); struct queue_limits lim; - int error; loff_t size; - bool partscan; - bool is_loop; - - if (!file) - return -EBADF; - - error =3D loop_check_backing_file(file); - if (error) { - fput(file); - return error; - } - - is_loop =3D is_loop_device(file); - - /* This is safe, since we have a reference from open(). */ - __module_get(THIS_MODULE); - - /* - * If we don't hold exclusive handle for the device, upgrade to it - * here to avoid changing device under exclusive owner. - */ - if (!(mode & BLK_OPEN_EXCL)) { - error =3D bd_prepare_to_claim(bdev, loop_configure, NULL); - if (error) - goto out_putf; - } - - error =3D loop_global_lock_killable(lo, is_loop); - if (error) - goto out_bdev; + int error; =20 - error =3D -EBUSY; if (lo->lo_state !=3D Lo_unbound) - goto out_unlock; + return -EBUSY; =20 - error =3D loop_validate_file(file, bdev); + error =3D loop_validate_file(lo, file, bdev); if (error) - goto out_unlock; + return error; =20 - if ((config->info.lo_flags & ~LOOP_CONFIGURE_SETTABLE_FLAGS) !=3D 0) { - error =3D -EINVAL; - goto out_unlock; - } + if ((config->info.lo_flags & ~LOOP_CONFIGURE_SETTABLE_FLAGS) !=3D 0) + return -EINVAL; =20 error =3D loop_set_status_from_info(lo, &config->info); if (error) - goto out_unlock; + return error; lo->lo_flags =3D config->info.lo_flags; =20 if (!(file->f_mode & FMODE_WRITE) || !(mode & BLK_OPEN_WRITE) || @@ -1065,10 +1104,8 @@ static int loop_configure(struct loop_device *lo, bl= k_mode_t mode, lo->workqueue =3D alloc_workqueue("loop%d", WQ_UNBOUND | WQ_FREEZABLE, 0, lo->lo_number); - if (!lo->workqueue) { - error =3D -ENOMEM; - goto out_unlock; - } + if (!lo->workqueue) + return -ENOMEM; } =20 /* suppress uevents while reconfiguring the device */ @@ -1085,7 +1122,7 @@ static int loop_configure(struct loop_device *lo, blk= _mode_t mode, /* No need to freeze the queue as the device isn't bound yet. */ error =3D queue_limits_commit_update(lo->lo_queue, &lim); if (error) - goto out_unlock; + return error; =20 /* * We might switch to direct I/O mode for the loop device, write back @@ -1100,20 +1137,69 @@ static int loop_configure(struct loop_device *lo, b= lk_mode_t mode, size =3D lo_calculate_size(lo, file); loop_set_size(lo, size); =20 - /* Order wrt reading lo_state in loop_validate_file(). */ - wmb(); - WRITE_ONCE(lo->lo_state, Lo_bound); if (part_shift) lo->lo_flags |=3D LO_FLAGS_PARTSCAN; - partscan =3D lo->lo_flags & LO_FLAGS_PARTSCAN; - if (partscan) + *partscan =3D lo->lo_flags & LO_FLAGS_PARTSCAN; + if (*partscan) clear_bit(GD_SUPPRESS_PART_SCAN, &lo->lo_disk->state); =20 dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 0); kobject_uevent(&disk_to_dev(lo->lo_disk)->kobj, KOBJ_CHANGE); =20 - loop_global_unlock(lo, is_loop); + return 0; +} + +static int loop_configure(struct loop_device *lo, blk_mode_t mode, + struct block_device *bdev, + const struct loop_config *config) +{ + struct file *file =3D fget(config->fd); + int error; + bool partscan; + bool is_loop; + + if (!file) + return -EBADF; + + error =3D loop_check_backing_file(file); + if (error) { + fput(file); + return error; + } + + is_loop =3D is_loop_device(file); + + /* This is safe, since we have a reference from open(). */ + __module_get(THIS_MODULE); + + /* + * If we don't hold exclusive handle for the device, upgrade to it + * here to avoid changing device under exclusive owner. + */ + if (!(mode & BLK_OPEN_EXCL)) { + error =3D bd_prepare_to_claim(bdev, loop_configure, NULL); + if (error) + goto out_putf; + } + + if (is_loop) { + error =3D loop_global_lock_killable(lo); + if (error) + goto out_bdev; + error =3D __loop_configure(lo, mode, bdev, config, file, + &partscan); + loop_global_unlock(lo); + } else { + error =3D mutex_lock_killable(&lo->lo_mutex); + if (error) + goto out_bdev; + error =3D __loop_configure(lo, mode, bdev, config, file, + &partscan); + mutex_unlock(&lo->lo_mutex); + } + if (error) + goto out_bdev; if (partscan) loop_reread_partitions(lo); =20 @@ -1122,8 +1208,6 @@ static int loop_configure(struct loop_device *lo, blk= _mode_t mode, =20 return 0; =20 -out_unlock: - loop_global_unlock(lo, is_loop); out_bdev: if (!(mode & BLK_OPEN_EXCL)) bd_abort_claiming(bdev, loop_configure); @@ -1139,29 +1223,27 @@ static void __loop_clr_fd(struct loop_device *lo) struct queue_limits lim; struct file *filp; gfp_t gfp =3D lo->old_gfp_mask; + unsigned int memflags; int err; =20 - spin_lock_irq(&lo->lo_lock); + mutex_lock(&lo->lo_mutex); + lim =3D queue_limits_start_update(lo->lo_queue); + memflags =3D blk_mq_freeze_queue(lo->lo_queue); filp =3D lo->lo_backing_file; lo->lo_backing_file =3D NULL; - spin_unlock_irq(&lo->lo_lock); =20 lo->lo_device =3D NULL; lo->lo_offset =3D 0; lo->lo_sizelimit =3D 0; memset(lo->lo_file_name, 0, LO_NAME_SIZE); =20 - /* - * Reset the block size to the default. - * - * No queue freezing needed because this is called from the final - * ->release call only, so there can't be any outstanding I/O. - */ - lim =3D queue_limits_start_update(lo->lo_queue); + /* Reset the block size to the default. */ lim.logical_block_size =3D SECTOR_SIZE; lim.physical_block_size =3D SECTOR_SIZE; lim.io_min =3D SECTOR_SIZE; queue_limits_commit_update(lo->lo_queue, &lim); + blk_mq_unfreeze_queue(lo->lo_queue, memflags); + mutex_unlock(&lo->lo_mutex); =20 invalidate_disk(lo->lo_disk); loop_sysfs_exit(lo); @@ -1220,11 +1302,11 @@ static int loop_clr_fd(struct loop_device *lo) * which loop_configure()/loop_change_fd() found via fget() was this * loop device. */ - err =3D loop_global_lock_killable(lo, true); + err =3D loop_global_lock_killable(lo); if (err) return err; if (lo->lo_state !=3D Lo_bound) { - loop_global_unlock(lo, true); + loop_global_unlock(lo); return -ENXIO; } /* @@ -1236,7 +1318,7 @@ static int loop_clr_fd(struct loop_device *lo) lo->lo_flags |=3D LO_FLAGS_AUTOCLEAR; if (disk_openers(lo->lo_disk) =3D=3D 1) WRITE_ONCE(lo->lo_state, Lo_rundown); - loop_global_unlock(lo, true); + loop_global_unlock(lo); =20 return 0; } @@ -1422,6 +1504,7 @@ loop_get_status64(struct loop_device *lo, struct loop= _info64 __user *arg) { } =20 static int loop_set_capacity(struct loop_device *lo) + __must_hold(&lo->lo_mutex) { loff_t size; =20 @@ -1435,6 +1518,7 @@ static int loop_set_capacity(struct loop_device *lo) } =20 static int loop_set_dio(struct loop_device *lo, unsigned long arg) + __must_hold(&lo->lo_mutex) { bool use_dio =3D !!arg; unsigned int memflags; @@ -1782,6 +1866,7 @@ static void lo_free_disk(struct gendisk *disk) loop_free_idle_workers(lo, true); timer_shutdown_sync(&lo->timer); mutex_destroy(&lo->lo_mutex); + lockdep_unregister_key(&lo->lo_mutex_key); kfree(lo); } =20 @@ -1966,13 +2051,15 @@ static void loop_handle_cmd(struct loop_cmd *cmd) } =20 static void loop_process_work(struct loop_worker *worker, - struct list_head *cmd_list, struct loop_device *lo) + struct loop_device *lo, bool rootcg) { int orig_flags =3D current->flags; + struct list_head *cmd_list; struct loop_cmd *cmd; =20 current->flags |=3D PF_LOCAL_THROTTLE | PF_MEMALLOC_NOIO; spin_lock_irq(&lo->lo_work_lock); + cmd_list =3D rootcg ? &lo->rootcg_cmd_list : &worker->cmd_list; while (!list_empty(cmd_list)) { cmd =3D container_of( cmd_list->next, struct loop_cmd, list_entry); @@ -2003,14 +2090,14 @@ static void loop_workfn(struct work_struct *work) { struct loop_worker *worker =3D container_of(work, struct loop_worker, work); - loop_process_work(worker, &worker->cmd_list, worker->lo); + loop_process_work(worker, worker->lo, false); } =20 static void loop_rootcg_workfn(struct work_struct *work) { struct loop_device *lo =3D container_of(work, struct loop_device, rootcg_work); - loop_process_work(NULL, &lo->rootcg_cmd_list, lo); + loop_process_work(NULL, lo, true); } =20 static const struct blk_mq_ops loop_mq_ops =3D { @@ -2034,10 +2121,10 @@ static int loop_add(int i) lo =3D kzalloc_obj(*lo); if (!lo) goto out; - lo->worker_tree =3D RB_ROOT; - INIT_LIST_HEAD(&lo->idle_worker_list); + context_unsafe(lo->worker_tree =3D RB_ROOT); + context_unsafe(INIT_LIST_HEAD(&lo->idle_worker_list)); timer_setup(&lo->timer, loop_free_idle_workers_timer, TIMER_DEFERRABLE); - WRITE_ONCE(lo->lo_state, Lo_unbound); + context_unsafe(WRITE_ONCE(lo->lo_state, Lo_unbound)); =20 err =3D mutex_lock_killable(&loop_ctl_mutex); if (err) @@ -2095,12 +2182,12 @@ static int loop_add(int i) */ if (!part_shift) set_bit(GD_SUPPRESS_PART_SCAN, &disk->state); - mutex_init(&lo->lo_mutex); + lockdep_register_key(&lo->lo_mutex_key); + mutex_init_with_key(&lo->lo_mutex, &lo->lo_mutex_key); lo->lo_number =3D i; - spin_lock_init(&lo->lo_lock); spin_lock_init(&lo->lo_work_lock); INIT_WORK(&lo->rootcg_work, loop_rootcg_workfn); - INIT_LIST_HEAD(&lo->rootcg_cmd_list); + context_unsafe(INIT_LIST_HEAD(&lo->rootcg_cmd_list)); disk->major =3D LOOP_MAJOR; disk->first_minor =3D i << part_shift; disk->minors =3D 1 << part_shift; @@ -2332,6 +2419,7 @@ static void __exit loop_exit(void) * module unloading is requested). If this is not a clean unloading, * we have no means to avoid kernel crash. */ + __assume_ctx_lock(&loop_ctl_mutex); idr_for_each_entry(&loop_index_idr, lo, id) loop_remove(lo); =20 --=20 2.55.0