From 94c4b4fd25e6c3763941bdec3ad54f2204afa992 Mon Sep 17 00:00:00 2001 From: Alistair Delva Date: Mon, 15 Nov 2021 18:16:55 +0000 Subject: [PATCH 1/7] block: Check ADMIN before NICE for IOPRIO_CLASS_RT Booting to Android userspace on 5.14 or newer triggers the following SELinux denial: avc: denied { sys_nice } for comm="init" capability=23 scontext=u:r:init:s0 tcontext=u:r:init:s0 tclass=capability permissive=0 Init is PID 0 running as root, so it already has CAP_SYS_ADMIN. For better compatibility with older SEPolicy, check ADMIN before NICE. Fixes: 9d3a39a5f1e4 ("block: grant IOPRIO_CLASS_RT to CAP_SYS_NICE") Signed-off-by: Alistair Delva Cc: Khazhismel Kumykov Cc: Bart Van Assche Cc: Serge Hallyn Cc: Jens Axboe Cc: Greg Kroah-Hartman Cc: Paul Moore Cc: selinux@vger.kernel.org Cc: linux-security-module@vger.kernel.org Cc: kernel-team@android.com Cc: stable@vger.kernel.org # v5.14+ Reviewed-by: Bart Van Assche Acked-by: Serge Hallyn Link: https://lore.kernel.org/r/20211115181655.3608659-1-adelva@google.com Signed-off-by: Jens Axboe --- block/ioprio.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/block/ioprio.c b/block/ioprio.c index 0e4ff245f2bf2..313c14a70bbd3 100644 --- a/block/ioprio.c +++ b/block/ioprio.c @@ -69,7 +69,14 @@ int ioprio_check_cap(int ioprio) switch (class) { case IOPRIO_CLASS_RT: - if (!capable(CAP_SYS_NICE) && !capable(CAP_SYS_ADMIN)) + /* + * Originally this only checked for CAP_SYS_ADMIN, + * which was implicitly allowed for pid 0 by security + * modules such as SELinux. Make sure we check + * CAP_SYS_ADMIN first to avoid a denial/avc for + * possibly missing CAP_SYS_NICE permission. + */ + if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_NICE)) return -EPERM; fallthrough; /* rt has prio field too */ From 95febeb61bf87ca803a1270498cd4cd61554a68f Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Mon, 15 Nov 2021 14:23:08 -0700 Subject: [PATCH 2/7] block: fix missing queue put in error path If we fail the submission queue checks, we don't put the queue afterwards. This can cause various issues like stalls on scheduler switch or failure to remove the device, or like in the original bug report, timeout waiting for the device on reboot/restart. While in there, fix a few whitespace discrepancies in the surrounding code. Link: https://bugzilla.kernel.org/show_bug.cgi?id=215039 Fixes: b637108a4022 ("blk-mq: fix filesystem I/O request allocation") Reported-and-tested-by: Stephen Smith Reviewed-by: Ming Lei Signed-off-by: Jens Axboe --- block/blk-mq.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/block/blk-mq.c b/block/blk-mq.c index 3ab34c4f20daf..5e1c9fd99353e 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -2543,8 +2543,7 @@ static struct request *blk_mq_get_new_requests(struct request_queue *q, return NULL; } -static inline bool blk_mq_can_use_cached_rq(struct request *rq, - struct bio *bio) +static inline bool blk_mq_can_use_cached_rq(struct request *rq, struct bio *bio) { if (blk_mq_get_hctx_type(bio->bi_opf) != rq->mq_hctx->type) return false; @@ -2565,7 +2564,6 @@ static inline struct request *blk_mq_get_request(struct request_queue *q, bool checked = false; if (plug) { - rq = rq_list_peek(&plug->cached_rq); if (rq && rq->q == q) { if (unlikely(!submit_bio_checks(bio))) @@ -2587,12 +2585,14 @@ static inline struct request *blk_mq_get_request(struct request_queue *q, fallback: if (unlikely(bio_queue_enter(bio))) return NULL; - if (!checked && !submit_bio_checks(bio)) - return NULL; + if (unlikely(!checked && !submit_bio_checks(bio))) + goto out_put; rq = blk_mq_get_new_requests(q, plug, bio, nsegs, same_queue_rq); - if (!rq) - blk_queue_exit(q); - return rq; + if (rq) + return rq; +out_put: + blk_queue_exit(q); + return NULL; } /** From 2a19b28f7929866e1cec92a3619f4de9f2d20005 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Tue, 16 Nov 2021 09:43:43 +0800 Subject: [PATCH 3/7] blk-mq: cancel blk-mq dispatch work in both blk_cleanup_queue and disk_release() For avoiding to slow down queue destroy, we don't call blk_mq_quiesce_queue() in blk_cleanup_queue(), instead of delaying to cancel dispatch work in blk_release_queue(). However, this way has caused kernel oops[1], reported by Changhui. The log shows that scsi_device can be freed before running blk_release_queue(), which is expected too since scsi_device is released after the scsi disk is closed and the scsi_device is removed. Fixes the issue by canceling blk-mq dispatch work in both blk_cleanup_queue() and disk_release(): 1) when disk_release() is run, the disk has been closed, and any sync dispatch activities have been done, so canceling dispatch work is enough to quiesce filesystem I/O dispatch activity. 2) in blk_cleanup_queue(), we only focus on passthrough request, and passthrough request is always explicitly allocated & freed by its caller, so once queue is frozen, all sync dispatch activity for passthrough request has been done, then it is enough to just cancel dispatch work for avoiding any dispatch activity. [1] kernel panic log [12622.769416] BUG: kernel NULL pointer dereference, address: 0000000000000300 [12622.777186] #PF: supervisor read access in kernel mode [12622.782918] #PF: error_code(0x0000) - not-present page [12622.788649] PGD 0 P4D 0 [12622.791474] Oops: 0000 [#1] PREEMPT SMP PTI [12622.796138] CPU: 10 PID: 744 Comm: kworker/10:1H Kdump: loaded Not tainted 5.15.0+ #1 [12622.804877] Hardware name: Dell Inc. PowerEdge R730/0H21J3, BIOS 1.5.4 10/002/2015 [12622.813321] Workqueue: kblockd blk_mq_run_work_fn [12622.818572] RIP: 0010:sbitmap_get+0x75/0x190 [12622.823336] Code: 85 80 00 00 00 41 8b 57 08 85 d2 0f 84 b1 00 00 00 45 31 e4 48 63 cd 48 8d 1c 49 48 c1 e3 06 49 03 5f 10 4c 8d 6b 40 83 f0 01 <48> 8b 33 44 89 f2 4c 89 ef 0f b6 c8 e8 fa f3 ff ff 83 f8 ff 75 58 [12622.844290] RSP: 0018:ffffb00a446dbd40 EFLAGS: 00010202 [12622.850120] RAX: 0000000000000001 RBX: 0000000000000300 RCX: 0000000000000004 [12622.858082] RDX: 0000000000000006 RSI: 0000000000000082 RDI: ffffa0b7a2dfe030 [12622.866042] RBP: 0000000000000004 R08: 0000000000000001 R09: ffffa0b742721334 [12622.874003] R10: 0000000000000008 R11: 0000000000000008 R12: 0000000000000000 [12622.881964] R13: 0000000000000340 R14: 0000000000000000 R15: ffffa0b7a2dfe030 [12622.889926] FS: 0000000000000000(0000) GS:ffffa0baafb40000(0000) knlGS:0000000000000000 [12622.898956] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [12622.905367] CR2: 0000000000000300 CR3: 0000000641210001 CR4: 00000000001706e0 [12622.913328] Call Trace: [12622.916055] [12622.918394] scsi_mq_get_budget+0x1a/0x110 [12622.922969] __blk_mq_do_dispatch_sched+0x1d4/0x320 [12622.928404] ? pick_next_task_fair+0x39/0x390 [12622.933268] __blk_mq_sched_dispatch_requests+0xf4/0x140 [12622.939194] blk_mq_sched_dispatch_requests+0x30/0x60 [12622.944829] __blk_mq_run_hw_queue+0x30/0xa0 [12622.949593] process_one_work+0x1e8/0x3c0 [12622.954059] worker_thread+0x50/0x3b0 [12622.958144] ? rescuer_thread+0x370/0x370 [12622.962616] kthread+0x158/0x180 [12622.966218] ? set_kthread_struct+0x40/0x40 [12622.970884] ret_from_fork+0x22/0x30 [12622.974875] [12622.977309] Modules linked in: scsi_debug rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache netfs sunrpc dm_multipath intel_rapl_msr intel_rapl_common dell_wmi_descriptor sb_edac rfkill video x86_pkg_temp_thermal intel_powerclamp dcdbas coretemp kvm_intel kvm mgag200 irqbypass i2c_algo_bit rapl drm_kms_helper ipmi_ssif intel_cstate intel_uncore syscopyarea sysfillrect sysimgblt fb_sys_fops pcspkr cec mei_me lpc_ich mei ipmi_si ipmi_devintf ipmi_msghandler acpi_power_meter drm fuse xfs libcrc32c sr_mod cdrom sd_mod t10_pi sg ixgbe ahci libahci crct10dif_pclmul crc32_pclmul crc32c_intel libata megaraid_sas ghash_clmulni_intel tg3 wdat_wdt mdio dca wmi dm_mirror dm_region_hash dm_log dm_mod [last unloaded: scsi_debug] Reported-by: ChanghuiZhong Cc: Christoph Hellwig Cc: "Martin K. Petersen" Cc: Bart Van Assche Cc: linux-scsi@vger.kernel.org Signed-off-by: Ming Lei Link: https://lore.kernel.org/r/20211116014343.610501-1-ming.lei@redhat.com Signed-off-by: Jens Axboe --- block/blk-core.c | 4 +++- block/blk-mq.c | 13 +++++++++++++ block/blk-mq.h | 2 ++ block/blk-sysfs.c | 10 ---------- block/genhd.c | 2 ++ 5 files changed, 20 insertions(+), 11 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index 9ee32f85d74e1..f0f38ca8e22f2 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -363,8 +363,10 @@ void blk_cleanup_queue(struct request_queue *q) blk_queue_flag_set(QUEUE_FLAG_DEAD, q); blk_sync_queue(q); - if (queue_is_mq(q)) + if (queue_is_mq(q)) { + blk_mq_cancel_work_sync(q); blk_mq_exit_queue(q); + } /* * In theory, request pool of sched_tags belongs to request queue. diff --git a/block/blk-mq.c b/block/blk-mq.c index 5e1c9fd99353e..eecbd7e6fea26 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -4417,6 +4417,19 @@ unsigned int blk_mq_rq_cpu(struct request *rq) } EXPORT_SYMBOL(blk_mq_rq_cpu); +void blk_mq_cancel_work_sync(struct request_queue *q) +{ + if (queue_is_mq(q)) { + struct blk_mq_hw_ctx *hctx; + int i; + + cancel_delayed_work_sync(&q->requeue_work); + + queue_for_each_hw_ctx(q, hctx, i) + cancel_delayed_work_sync(&hctx->run_work); + } +} + static int __init blk_mq_init(void) { int i; diff --git a/block/blk-mq.h b/block/blk-mq.h index 8acfa650f5751..afcf9931a4890 100644 --- a/block/blk-mq.h +++ b/block/blk-mq.h @@ -128,6 +128,8 @@ extern void blk_mq_hctx_kobj_init(struct blk_mq_hw_ctx *hctx); void blk_mq_free_plug_rqs(struct blk_plug *plug); void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule); +void blk_mq_cancel_work_sync(struct request_queue *q); + void blk_mq_release(struct request_queue *q); static inline struct blk_mq_ctx *__blk_mq_get_ctx(struct request_queue *q, diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c index cef1f713370bd..cd75b0f73dc6f 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -791,16 +791,6 @@ static void blk_release_queue(struct kobject *kobj) blk_free_queue_stats(q->stats); - if (queue_is_mq(q)) { - struct blk_mq_hw_ctx *hctx; - int i; - - cancel_delayed_work_sync(&q->requeue_work); - - queue_for_each_hw_ctx(q, hctx, i) - cancel_delayed_work_sync(&hctx->run_work); - } - blk_exit_queue(q); blk_queue_free_zone_bitmaps(q); diff --git a/block/genhd.c b/block/genhd.c index c5392cc24d37e..30362aeacac4b 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -1111,6 +1111,8 @@ static void disk_release(struct device *dev) might_sleep(); WARN_ON_ONCE(disk_live(disk)); + blk_mq_cancel_work_sync(disk->queue); + disk_release_events(disk); kfree(disk->random); xa_destroy(&disk->part_tbl); From d1faacbf67b1944f0e0c618dc581d929263f6fe9 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 16 Nov 2021 10:15:59 -0800 Subject: [PATCH 4/7] Revert "mark pstore-blk as broken" This reverts commit d07f3b081ee632268786601f55e1334d1f68b997. pstore-blk was fixed to avoid the unwanted APIs in commit 7bb9557b48fc ("pstore/blk: Use the normal block device I/O path"), which landed in the same release as the commit adding BROKEN. Cc: Jens Axboe Cc: Christoph Hellwig Cc: stable@vger.kernel.org Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20211116181559.3975566-1-keescook@chromium.org Signed-off-by: Jens Axboe --- fs/pstore/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/pstore/Kconfig b/fs/pstore/Kconfig index 328da35da3908..8adabde685f13 100644 --- a/fs/pstore/Kconfig +++ b/fs/pstore/Kconfig @@ -173,7 +173,6 @@ config PSTORE_BLK tristate "Log panic/oops to a block device" depends on PSTORE depends on BLOCK - depends on BROKEN select PSTORE_ZONE default n help From 245a489e81e13dd55ae46d27becf6d5901eb7828 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Wed, 17 Nov 2021 19:55:02 +0800 Subject: [PATCH 5/7] block: avoid to quiesce queue in elevator_init_mq elevator_init_mq() is only called before adding disk, when there isn't any FS I/O, only passthrough requests can be queued, so freezing queue plus canceling dispatch work is enough to drain any dispatch activities, then we can avoid synchronize_srcu() in blk_mq_quiesce_queue(). Long boot latency issue can be fixed in case of lots of disks added during booting. Fixes: 737eb78e82d5 ("block: Delay default elevator initialization") Reported-by: yangerkun Cc: Damien Le Moal Signed-off-by: Ming Lei Link: https://lore.kernel.org/r/20211117115502.1600950-1-ming.lei@redhat.com Signed-off-by: Jens Axboe --- block/elevator.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/block/elevator.c b/block/elevator.c index 1f39f6e8ebb96..19a78d5516ba7 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -694,12 +694,18 @@ void elevator_init_mq(struct request_queue *q) if (!e) return; + /* + * We are called before adding disk, when there isn't any FS I/O, + * so freezing queue plus canceling dispatch work is enough to + * drain any dispatch activities originated from passthrough + * requests, then no need to quiesce queue which may add long boot + * latency, especially when lots of disks are involved. + */ blk_mq_freeze_queue(q); - blk_mq_quiesce_queue(q); + blk_mq_cancel_work_sync(q); err = blk_mq_init_sched(q, e); - blk_mq_unquiesce_queue(q); blk_mq_unfreeze_queue(q); if (err) { From 15c30104965101b8e76b24d27035569d6613a7d6 Mon Sep 17 00:00:00 2001 From: Yu Kuai Date: Tue, 2 Nov 2021 10:07:05 +0800 Subject: [PATCH 6/7] blk-cgroup: fix missing put device in error path from blkg_conf_pref() If blk_queue_enter() failed due to queue is dying, the blkdev_put_no_open() is needed because blkcg_conf_open_bdev() succeeded. Fixes: 0c9d338c8443 ("blk-cgroup: synchronize blkg creation against policy deactivation") Signed-off-by: Yu Kuai Acked-by: Tejun Heo Link: https://lore.kernel.org/r/20211102020705.2321858-1-yukuai3@huawei.com Signed-off-by: Jens Axboe --- block/blk-cgroup.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 88b1fce905200..663aabfeba183 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -640,7 +640,7 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol, */ ret = blk_queue_enter(q, 0); if (ret) - return ret; + goto fail; rcu_read_lock(); spin_lock_irq(&q->queue_lock); @@ -676,13 +676,13 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol, new_blkg = blkg_alloc(pos, q, GFP_KERNEL); if (unlikely(!new_blkg)) { ret = -ENOMEM; - goto fail; + goto fail_exit_queue; } if (radix_tree_preload(GFP_KERNEL)) { blkg_free(new_blkg); ret = -ENOMEM; - goto fail; + goto fail_exit_queue; } rcu_read_lock(); @@ -722,9 +722,10 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol, fail_unlock: spin_unlock_irq(&q->queue_lock); rcu_read_unlock(); +fail_exit_queue: + blk_queue_exit(q); fail: blkdev_put_no_open(bdev); - blk_queue_exit(q); /* * If queue was bypassing, we should retry. Do so after a * short msleep(). It isn't strictly necessary but queue From 2b504bd4841bccbf3eb83c1fec229b65956ad8ad Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Thu, 18 Nov 2021 23:30:41 +0800 Subject: [PATCH 7/7] blk-mq: don't insert FUA request with data into scheduler queue We never insert flush request into scheduler queue before. Recently commit d92ca9d8348f ("blk-mq: don't handle non-flush requests in blk_insert_flush") tries to handle FUA data request as normal request. This way has caused warning[1] in mq-deadline dd_exit_sched() or io hang in case of kyber since RQF_ELVPRIV isn't set for flush request, then ->finish_request won't be called. Fix the issue by inserting FUA data request with blk_mq_request_bypass_insert() when the device supports FUA, just like what we did before. [1] https://lore.kernel.org/linux-block/CAHj4cs-_vkTW=dAzbZYGxpEWSpzpcmaNeY1R=vH311+9vMUSdg@mail.gmail.com/ Reported-by: Yi Zhang Fixes: d92ca9d8348f ("blk-mq: don't handle non-flush requests in blk_insert_flush") Cc: Christoph Hellwig Signed-off-by: Ming Lei Reviewed-by: Bart Van Assche Link: https://lore.kernel.org/r/20211118153041.2163228-1-ming.lei@redhat.com Signed-off-by: Jens Axboe --- block/blk-flush.c | 12 ++++++------ block/blk-mq.c | 4 +++- block/blk.h | 2 +- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/block/blk-flush.c b/block/blk-flush.c index 8e364bda51661..1fce6d16e6d3a 100644 --- a/block/blk-flush.c +++ b/block/blk-flush.c @@ -379,7 +379,7 @@ static void mq_flush_data_end_io(struct request *rq, blk_status_t error) * @rq is being submitted. Analyze what needs to be done and put it on the * right queue. */ -bool blk_insert_flush(struct request *rq) +void blk_insert_flush(struct request *rq) { struct request_queue *q = rq->q; unsigned long fflags = q->queue_flags; /* may change, cache */ @@ -409,7 +409,7 @@ bool blk_insert_flush(struct request *rq) */ if (!policy) { blk_mq_end_request(rq, 0); - return true; + return; } BUG_ON(rq->bio != rq->biotail); /*assumes zero or single bio rq */ @@ -420,8 +420,10 @@ bool blk_insert_flush(struct request *rq) * for normal execution. */ if ((policy & REQ_FSEQ_DATA) && - !(policy & (REQ_FSEQ_PREFLUSH | REQ_FSEQ_POSTFLUSH))) - return false; + !(policy & (REQ_FSEQ_PREFLUSH | REQ_FSEQ_POSTFLUSH))) { + blk_mq_request_bypass_insert(rq, false, true); + return; + } /* * @rq should go through flush machinery. Mark it part of flush @@ -437,8 +439,6 @@ bool blk_insert_flush(struct request *rq) spin_lock_irq(&fq->mq_flush_lock); blk_flush_complete_seq(rq, fq, REQ_FSEQ_ACTIONS & ~policy, 0); spin_unlock_irq(&fq->mq_flush_lock); - - return true; } /** diff --git a/block/blk-mq.c b/block/blk-mq.c index eecbd7e6fea26..8799fa73ef348 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -2647,8 +2647,10 @@ void blk_mq_submit_bio(struct bio *bio) return; } - if (op_is_flush(bio->bi_opf) && blk_insert_flush(rq)) + if (op_is_flush(bio->bi_opf)) { + blk_insert_flush(rq); return; + } if (plug && (q->nr_hw_queues == 1 || blk_mq_is_shared_tags(rq->mq_hctx->flags) || diff --git a/block/blk.h b/block/blk.h index b4fed2033e48f..ccde6e6f17360 100644 --- a/block/blk.h +++ b/block/blk.h @@ -271,7 +271,7 @@ void __blk_account_io_done(struct request *req, u64 now); */ #define ELV_ON_HASH(rq) ((rq)->rq_flags & RQF_HASHED) -bool blk_insert_flush(struct request *rq); +void blk_insert_flush(struct request *rq); int elevator_switch_mq(struct request_queue *q, struct elevator_type *new_e);