From 744eac783f9e105358eed05b42dcc5c5789744b3 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sat, 28 Oct 2023 22:00:40 -0700 Subject: [PATCH 01/14] nvme-auth: use crypto_shash_tfm_digest() Simplify nvme_auth_augmented_challenge() by using crypto_shash_tfm_digest() instead of an alloc+init+update+final sequence. This should also improve performance. Signed-off-by: Eric Biggers Reviewed-by: Christoph Hellwig Signed-off-by: Keith Busch --- drivers/nvme/common/auth.c | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c index a8e87dfbeab2..a23ab5c968b9 100644 --- a/drivers/nvme/common/auth.c +++ b/drivers/nvme/common/auth.c @@ -341,7 +341,6 @@ int nvme_auth_augmented_challenge(u8 hmac_id, u8 *skey, size_t skey_len, u8 *challenge, u8 *aug, size_t hlen) { struct crypto_shash *tfm; - struct shash_desc *desc; u8 *hashed_key; const char *hmac_name; int ret; @@ -369,29 +368,11 @@ int nvme_auth_augmented_challenge(u8 hmac_id, u8 *skey, size_t skey_len, goto out_free_key; } - desc = kmalloc(sizeof(struct shash_desc) + crypto_shash_descsize(tfm), - GFP_KERNEL); - if (!desc) { - ret = -ENOMEM; - goto out_free_hash; - } - desc->tfm = tfm; - ret = crypto_shash_setkey(tfm, hashed_key, hlen); if (ret) - goto out_free_desc; + goto out_free_hash; - ret = crypto_shash_init(desc); - if (ret) - goto out_free_desc; - - ret = crypto_shash_update(desc, challenge, hlen); - if (ret) - goto out_free_desc; - - ret = crypto_shash_final(desc, aug); -out_free_desc: - kfree_sensitive(desc); + ret = crypto_shash_tfm_digest(tfm, challenge, hlen, aug); out_free_hash: crypto_free_shash(tfm); out_free_key: From 0e32fdd7968eb9a39aa4d4111aef0fda8684af9e Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Mon, 30 Oct 2023 15:49:28 +0100 Subject: [PATCH 02/14] nvme-tcp: Fix a memory leak All error handling path end to the error handling path, except this one. Go to the error handling branch as well here, otherwise 'icreq' and 'icresp' will leak. Fixes: 2837966ab2a8 ("nvme-tcp: control message handling for recvmsg()") Signed-off-by: Christophe JAILLET Reviewed-by: Christoph Hellwig Signed-off-by: Keith Busch --- drivers/nvme/host/tcp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index 4714a902f4ca..f97711fc9f9f 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -1423,13 +1423,14 @@ static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue) nvme_tcp_queue_id(queue), ret); goto free_icresp; } + ret = -ENOTCONN; if (queue->ctrl->ctrl.opts->tls) { ctype = tls_get_record_type(queue->sock->sk, (struct cmsghdr *)cbuf); if (ctype != TLS_RECORD_TYPE_DATA) { pr_err("queue %d: unhandled TLS record %d\n", nvme_tcp_queue_id(queue), ctype); - return -ENOTCONN; + goto free_icresp; } } ret = -EINVAL; From 983a338b96c8a25b81e773b643f80634358e81bc Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Mon, 30 Oct 2023 17:00:44 +0100 Subject: [PATCH 03/14] nvme: update firmware version after commit The firmware version sysfs entry needs to be updated after a successfully firmware activation. nvme-cli stopped issuing an Identify Controller command to list the current firmware information and relies on sysfs showing the current firmware version. Reported-by: Kenji Tomonaga Signed-off-by: Daniel Wagner Tested-by: Kenji Tomonaga Reviewed-by: Christoph Hellwig Reviewed-by: Niklas Cassel [fixed off-by one afi index] Signed-off-by: Keith Busch --- drivers/nvme/host/core.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 62612f87aafa..97441d022926 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -4074,8 +4074,21 @@ static void nvme_get_fw_slot_info(struct nvme_ctrl *ctrl) return; if (nvme_get_log(ctrl, NVME_NSID_ALL, NVME_LOG_FW_SLOT, 0, NVME_CSI_NVM, - log, sizeof(*log), 0)) + log, sizeof(*log), 0)) { dev_warn(ctrl->device, "Get FW SLOT INFO log error\n"); + goto out_free_log; + } + + if (log->afi & 0x70 || !(log->afi & 0x7)) { + dev_info(ctrl->device, + "Firmware is activated after next Controller Level Reset\n"); + goto out_free_log; + } + + memcpy(ctrl->subsys->firmware_rev, &log->frs[(log->afi & 0x7) - 1], + sizeof(ctrl->subsys->firmware_rev)); + +out_free_log: kfree(log); } From 1147dd0503564fa0e03489a039f9e0c748a03db4 Mon Sep 17 00:00:00 2001 From: Anuj Gupta Date: Thu, 19 Oct 2023 00:54:30 +0530 Subject: [PATCH 04/14] nvme: fix error-handling for io_uring nvme-passthrough Driver may return an error before submitting the command to the device. Ensure that such error is propagated up. Fixes: 456cba386e94 ("nvme: wire-up uring-cmd support for io-passthru on char-device.") Signed-off-by: Anuj Gupta Signed-off-by: Kanchan Joshi Reviewed-by: Niklas Cassel Reviewed-by: Christoph Hellwig Signed-off-by: Keith Busch --- drivers/nvme/host/ioctl.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c index 747c879e8982..529b9954d2b8 100644 --- a/drivers/nvme/host/ioctl.c +++ b/drivers/nvme/host/ioctl.c @@ -510,10 +510,13 @@ static enum rq_end_io_ret nvme_uring_cmd_end_io(struct request *req, struct nvme_uring_cmd_pdu *pdu = nvme_uring_cmd_pdu(ioucmd); req->bio = pdu->bio; - if (nvme_req(req)->flags & NVME_REQ_CANCELLED) + if (nvme_req(req)->flags & NVME_REQ_CANCELLED) { pdu->nvme_status = -EINTR; - else + } else { pdu->nvme_status = nvme_req(req)->status; + if (!pdu->nvme_status) + pdu->nvme_status = blk_status_to_errno(err); + } pdu->u.result = le64_to_cpu(nvme_req(req)->result.u64); /* From 75276847f4e262a52ccaf1a1c6b929280ddf77f6 Mon Sep 17 00:00:00 2001 From: Mark O'Donovan Date: Wed, 25 Oct 2023 10:51:23 +0000 Subject: [PATCH 05/14] nvme-auth: auth success1 msg always includes resp In cases where RVALID is false, the response is still transmitted, but is cleared to zero. Relevant extract from the spec: Response R2, if valid (i.e., if the RVALID field is set to 01h), cleared to 0h otherwise Signed-off-by: Mark O'Donovan Reviewed-by: Hannes Reinecke Reviewed-by: Sagi Grimberg Reviewed-by: Christoph Hellwig --- drivers/nvme/host/auth.c | 5 +---- include/linux/nvme.h | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c index eaefebb2a799..5885bb0d5a9a 100644 --- a/drivers/nvme/host/auth.c +++ b/drivers/nvme/host/auth.c @@ -339,10 +339,7 @@ static int nvme_auth_process_dhchap_success1(struct nvme_ctrl *ctrl, struct nvme_dhchap_queue_context *chap) { struct nvmf_auth_dhchap_success1_data *data = chap->buf; - size_t size = sizeof(*data); - - if (chap->s2) - size += chap->hash_len; + size_t size = sizeof(*data) + chap->hash_len; if (size > CHAP_BUF_SIZE) { chap->status = NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD; diff --git a/include/linux/nvme.h b/include/linux/nvme.h index a7ba74babad7..44325c068b6a 100644 --- a/include/linux/nvme.h +++ b/include/linux/nvme.h @@ -1732,7 +1732,7 @@ struct nvmf_auth_dhchap_success1_data { __u8 rsvd2; __u8 rvalid; __u8 rsvd3[7]; - /* 'hl' bytes of response value if 'rvalid' is set */ + /* 'hl' bytes of response value */ __u8 rval[]; }; From fc1e03eacac8e5ff8664ce2ebadabba8604f09f4 Mon Sep 17 00:00:00 2001 From: Mark O'Donovan Date: Wed, 25 Oct 2023 10:51:24 +0000 Subject: [PATCH 06/14] nvme-auth: add flag for bi-directional auth Introduces an explicit variable for bi-directional auth. The currently used variable chap->s2 is incorrectly zeroed for uni-directional auth. That will be fixed in the next patch so this needs to change to avoid sending unexpected success2 messages Signed-off-by: Mark O'Donovan Reviewed-by: Christoph Hellwig Reviewed-by: Hannes Reinecke --- drivers/nvme/host/auth.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c index 5885bb0d5a9a..bff2b2c3cd79 100644 --- a/drivers/nvme/host/auth.c +++ b/drivers/nvme/host/auth.c @@ -29,6 +29,7 @@ struct nvme_dhchap_queue_context { int error; u32 s1; u32 s2; + bool bi_directional; u16 transaction; u8 status; u8 dhgroup_id; @@ -312,6 +313,7 @@ static int nvme_auth_set_dhchap_reply_data(struct nvme_ctrl *ctrl, data->dhvlen = cpu_to_le16(chap->host_key_len); memcpy(data->rval, chap->response, chap->hash_len); if (ctrl->ctrl_key) { + chap->bi_directional = true; get_random_bytes(chap->c2, chap->hash_len); data->cvalid = 1; chap->s2 = nvme_auth_get_seqnum(); @@ -660,6 +662,7 @@ static void nvme_auth_reset_dhchap(struct nvme_dhchap_queue_context *chap) chap->error = 0; chap->s1 = 0; chap->s2 = 0; + chap->bi_directional = false; chap->transaction = 0; memset(chap->c1, 0, sizeof(chap->c1)); memset(chap->c2, 0, sizeof(chap->c2)); @@ -822,7 +825,7 @@ static void nvme_queue_auth_work(struct work_struct *work) goto fail2; } - if (chap->s2) { + if (chap->bi_directional) { /* DH-HMAC-CHAP Step 5: send success2 */ dev_dbg(ctrl->device, "%s: qid %d send success2\n", __func__, chap->qid); From 6f66d046eade7a5b979e349ac52026ddfe2776b3 Mon Sep 17 00:00:00 2001 From: Mark O'Donovan Date: Wed, 25 Oct 2023 10:51:25 +0000 Subject: [PATCH 07/14] nvme-auth: always set valid seq_num in dhchap reply Currently a seqnum of zero is sent during uni-directional authentication. The zero value is reserved for the secure channel feature which is not yet implemented. Relevant extract from the spec: The value 0h is used to indicate that bidirectional authentication is not performed, but a challenge value C2 is carried in order to generate a pre-shared key (PSK) for subsequent establishment of a secure channel Signed-off-by: Mark O'Donovan Reviewed-by: Christoph Hellwig Reviewed-by: Hannes Reinecke --- drivers/nvme/host/auth.c | 3 +-- drivers/nvme/target/fabrics-cmd-auth.c | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c index bff2b2c3cd79..48328e36e93b 100644 --- a/drivers/nvme/host/auth.c +++ b/drivers/nvme/host/auth.c @@ -316,15 +316,14 @@ static int nvme_auth_set_dhchap_reply_data(struct nvme_ctrl *ctrl, chap->bi_directional = true; get_random_bytes(chap->c2, chap->hash_len); data->cvalid = 1; - chap->s2 = nvme_auth_get_seqnum(); memcpy(data->rval + chap->hash_len, chap->c2, chap->hash_len); dev_dbg(ctrl->device, "%s: qid %d ctrl challenge %*ph\n", __func__, chap->qid, (int)chap->hash_len, chap->c2); } else { memset(chap->c2, 0, chap->hash_len); - chap->s2 = 0; } + chap->s2 = nvme_auth_get_seqnum(); data->seqnum = cpu_to_le32(chap->s2); if (chap->host_key_len) { dev_dbg(ctrl->device, "%s: qid %d host public key %*ph\n", diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c index 1d9854484e2e..eb7785be0ca7 100644 --- a/drivers/nvme/target/fabrics-cmd-auth.c +++ b/drivers/nvme/target/fabrics-cmd-auth.c @@ -163,11 +163,11 @@ static u16 nvmet_auth_reply(struct nvmet_req *req, void *d) pr_debug("%s: ctrl %d qid %d challenge %*ph\n", __func__, ctrl->cntlid, req->sq->qid, data->hl, req->sq->dhchap_c2); - req->sq->dhchap_s2 = le32_to_cpu(data->seqnum); } else { req->sq->authenticated = true; req->sq->dhchap_c2 = NULL; } + req->sq->dhchap_s2 = le32_to_cpu(data->seqnum); return 0; } From fd1418de10b9ca03d78404cf00a95138689ea369 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Tue, 24 Oct 2023 08:13:35 +0200 Subject: [PATCH 08/14] nvme-tcp: avoid open-coding nvme_tcp_teardown_admin_queue() nvme_tcp_setup_ctrl() has an open-coded version of nvme_tcp_teardown_admin_queue(). Signed-off-by: Hannes Reinecke Tested-by: Mark O'Donovan Signed-off-by: Keith Busch --- drivers/nvme/host/tcp.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index f97711fc9f9f..89661a9cf850 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -2237,11 +2237,7 @@ static int nvme_tcp_setup_ctrl(struct nvme_ctrl *ctrl, bool new) nvme_tcp_destroy_io_queues(ctrl, new); } destroy_admin: - nvme_quiesce_admin_queue(ctrl); - blk_sync_queue(ctrl->admin_q); - nvme_tcp_stop_queue(ctrl, 0); - nvme_cancel_admin_tagset(ctrl); - nvme_tcp_destroy_admin_queue(ctrl, new); + nvme_tcp_teardown_admin_queue(ctrl, false); return ret; } From 55adcdbbdd349de935de677ccb59ff8be8c67f6a Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Tue, 24 Oct 2023 08:13:36 +0200 Subject: [PATCH 09/14] nvme-loop: always quiesce and cancel commands before destroying admin q Once ->init_ctrl_finish() is called there may be commands outstanding, so we should quiesce the admin queue and cancel all commands prior to call nvme_loop_destroy_admin_queue(). Signed-off-by: Hannes Reinecke Tested-by: Mark O'Donovan Signed-off-by: Keith Busch --- drivers/nvme/target/loop.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c index 48d5df054cd0..9cb434c58075 100644 --- a/drivers/nvme/target/loop.c +++ b/drivers/nvme/target/loop.c @@ -466,6 +466,8 @@ static void nvme_loop_reset_ctrl_work(struct work_struct *work) out_destroy_io: nvme_loop_destroy_io_queues(ctrl); out_destroy_admin: + nvme_quiesce_admin_queue(&ctrl->ctrl); + nvme_cancel_admin_tagset(&ctrl->ctrl); nvme_loop_destroy_admin_queue(ctrl); out_disable: dev_warn(ctrl->ctrl.device, "Removing after reset failure\n"); @@ -600,6 +602,8 @@ static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev, return &ctrl->ctrl; out_remove_admin_queue: + nvme_quiesce_admin_queue(&ctrl->ctrl); + nvme_cancel_admin_tagset(&ctrl->ctrl); nvme_loop_destroy_admin_queue(ctrl); out_free_queues: kfree(ctrl->queues); From 4733b65d82bdb19bca5ba47ff6c9b24bce1b3f9f Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Tue, 24 Oct 2023 08:13:37 +0200 Subject: [PATCH 10/14] nvme: start keep-alive after admin queue setup Setting up I/O queues might take quite some time on larger and/or busy setups, so KATO might expire before all I/O queues could be set up. Fix this by start keep alive from the ->init_ctrl_finish() callback, and stopping it when calling nvme_cancel_admin_tagset(). Signed-off-by: Hannes Reinecke Tested-by: Mark O'Donovan [fixed nvme-fc compile error] Signed-off-by: Keith Busch --- drivers/nvme/host/core.c | 6 +++--- drivers/nvme/host/fc.c | 6 ++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 97441d022926..75a1b58a7a43 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -483,6 +483,7 @@ EXPORT_SYMBOL_GPL(nvme_cancel_tagset); void nvme_cancel_admin_tagset(struct nvme_ctrl *ctrl) { + nvme_stop_keep_alive(ctrl); if (ctrl->admin_tagset) { blk_mq_tagset_busy_iter(ctrl->admin_tagset, nvme_cancel_request, ctrl); @@ -3200,6 +3201,8 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl, bool was_suspended) clear_bit(NVME_CTRL_DIRTY_CAPABILITY, &ctrl->flags); ctrl->identified = true; + nvme_start_keep_alive(ctrl); + return 0; } EXPORT_SYMBOL_GPL(nvme_init_ctrl_finish); @@ -4346,7 +4349,6 @@ void nvme_stop_ctrl(struct nvme_ctrl *ctrl) { nvme_mpath_stop(ctrl); nvme_auth_stop(ctrl); - nvme_stop_keep_alive(ctrl); nvme_stop_failfast_work(ctrl); flush_work(&ctrl->async_event_work); cancel_work_sync(&ctrl->fw_act_work); @@ -4357,8 +4359,6 @@ EXPORT_SYMBOL_GPL(nvme_stop_ctrl); void nvme_start_ctrl(struct nvme_ctrl *ctrl) { - nvme_start_keep_alive(ctrl); - nvme_enable_aen(ctrl); /* diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c index a15b37750d6e..49c3e46eaa1e 100644 --- a/drivers/nvme/host/fc.c +++ b/drivers/nvme/host/fc.c @@ -2530,6 +2530,12 @@ __nvme_fc_abort_outstanding_ios(struct nvme_fc_ctrl *ctrl, bool start_queues) * clean up the admin queue. Same thing as above. */ nvme_quiesce_admin_queue(&ctrl->ctrl); + + /* + * Open-coding nvme_cancel_admin_tagset() as fc + * is not using nvme_cancel_request(). + */ + nvme_stop_keep_alive(&ctrl->ctrl); blk_sync_queue(ctrl->ctrl.admin_q); blk_mq_tagset_busy_iter(&ctrl->admin_tag_set, nvme_fc_terminate_exchange, &ctrl->ctrl); From 327462725b0f759f093788dfbcb2f1fd132f956b Mon Sep 17 00:00:00 2001 From: Li Lingfeng Date: Tue, 7 Nov 2023 18:34:35 +0800 Subject: [PATCH 11/14] nbd: fix uaf in nbd_open Commit 4af5f2e03013 ("nbd: use blk_mq_alloc_disk and blk_cleanup_disk") cleans up disk by blk_cleanup_disk() and it won't set disk->private_data as NULL as before. UAF may be triggered in nbd_open() if someone tries to open nbd device right after nbd_put() since nbd has been free in nbd_dev_remove(). Fix this by implementing ->free_disk and free private data in it. Fixes: 4af5f2e03013 ("nbd: use blk_mq_alloc_disk and blk_cleanup_disk") Signed-off-by: Li Lingfeng Reviewed-by: Josef Bacik Link: https://lore.kernel.org/r/20231107103435.2074904-1-lilingfeng@huaweicloud.com Signed-off-by: Jens Axboe --- drivers/block/nbd.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 800f131222fc..855fdf5c3b4e 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -250,7 +250,6 @@ static void nbd_dev_remove(struct nbd_device *nbd) struct gendisk *disk = nbd->disk; del_gendisk(disk); - put_disk(disk); blk_mq_free_tag_set(&nbd->tag_set); /* @@ -261,7 +260,7 @@ static void nbd_dev_remove(struct nbd_device *nbd) idr_remove(&nbd_index_idr, nbd->index); mutex_unlock(&nbd_index_mutex); destroy_workqueue(nbd->recv_workq); - kfree(nbd); + put_disk(disk); } static void nbd_dev_remove_work(struct work_struct *work) @@ -1608,6 +1607,13 @@ static void nbd_release(struct gendisk *disk) nbd_put(nbd); } +static void nbd_free_disk(struct gendisk *disk) +{ + struct nbd_device *nbd = disk->private_data; + + kfree(nbd); +} + static const struct block_device_operations nbd_fops = { .owner = THIS_MODULE, @@ -1615,6 +1621,7 @@ static const struct block_device_operations nbd_fops = .release = nbd_release, .ioctl = nbd_ioctl, .compat_ioctl = nbd_ioctl, + .free_disk = nbd_free_disk, }; #if IS_ENABLED(CONFIG_DEBUG_FS) From 1b0a151c10a6d823f033023b9fdd9af72a89591b Mon Sep 17 00:00:00 2001 From: Yu Kuai Date: Tue, 7 Nov 2023 19:12:47 +0800 Subject: [PATCH 12/14] blk-core: use pr_warn_ratelimited() in bio_check_ro() If one of the underlying disks of raid or dm is set to read-only, then each io will generate new log, which will cause message storm. This environment is indeed problematic, however we can't make sure our naive custormer won't do this, hence use pr_warn_ratelimited() to prevent message storm in this case. Signed-off-by: Yu Kuai Fixes: 57e95e4670d1 ("block: fix and cleanup bio_check_ro") Signed-off-by: Ye Bin Link: https://lore.kernel.org/r/20231107111247.2157820-1-yukuai1@huaweicloud.com Signed-off-by: Jens Axboe --- block/blk-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index 9d51e9894ece..fdf25b8d6e78 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -501,8 +501,8 @@ static inline void bio_check_ro(struct bio *bio) if (op_is_write(bio_op(bio)) && bdev_read_only(bio->bi_bdev)) { if (op_is_flush(bio->bi_opf) && !bio_sectors(bio)) return; - pr_warn("Trying to write to read-only block-device %pg\n", - bio->bi_bdev); + pr_warn_ratelimited("Trying to write to read-only block-device %pg\n", + bio->bi_bdev); /* Older lvm-tools actually trigger this */ } } From 6affe08aea5f3b630565676e227b41d55a6f009c Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Thu, 26 Oct 2023 15:08:03 +0200 Subject: [PATCH 13/14] nvme: common: make keyring and auth separate modules When only the keyring module is included but auth is not, modpost complains about the lack of a module license tag: ERROR: modpost: missing MODULE_LICENSE() in drivers/nvme/common/nvme-common.o Address this by making both modules buildable standalone, removing the now unnecessary CONFIG_NVME_COMMON symbol in the process. Also, now that NVME_KEYRING config symbol can be either a module or built-in, the stubs need to check for '#if IS_ENABLED' rather than a simple '#ifdef'. Fixes: 9d77eb5277849 ("nvme-keyring: register '.nvme' keyring") Signed-off-by: Arnd Bergmann Signed-off-by: Hannes Reinecke Signed-off-by: Christoph Hellwig Signed-off-by: Keith Busch --- drivers/nvme/Makefile | 2 +- drivers/nvme/common/Kconfig | 7 ++----- drivers/nvme/common/Makefile | 7 ++++--- drivers/nvme/common/keyring.c | 2 ++ drivers/nvme/host/Kconfig | 2 -- drivers/nvme/target/Kconfig | 2 -- include/linux/nvme-keyring.h | 2 +- 7 files changed, 10 insertions(+), 14 deletions(-) diff --git a/drivers/nvme/Makefile b/drivers/nvme/Makefile index eedca8c72098..74f59ceed3d5 100644 --- a/drivers/nvme/Makefile +++ b/drivers/nvme/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0-only -obj-$(CONFIG_NVME_COMMON) += common/ +obj-y += common/ obj-y += host/ obj-y += target/ diff --git a/drivers/nvme/common/Kconfig b/drivers/nvme/common/Kconfig index 06c8df00d1e2..244432e0b73d 100644 --- a/drivers/nvme/common/Kconfig +++ b/drivers/nvme/common/Kconfig @@ -1,14 +1,11 @@ # SPDX-License-Identifier: GPL-2.0-only -config NVME_COMMON - tristate - config NVME_KEYRING - bool + tristate select KEYS config NVME_AUTH - bool + tristate select CRYPTO select CRYPTO_HMAC select CRYPTO_SHA256 diff --git a/drivers/nvme/common/Makefile b/drivers/nvme/common/Makefile index 0cbd0b0b8d49..681514cf2e2f 100644 --- a/drivers/nvme/common/Makefile +++ b/drivers/nvme/common/Makefile @@ -2,7 +2,8 @@ ccflags-y += -I$(src) -obj-$(CONFIG_NVME_COMMON) += nvme-common.o +obj-$(CONFIG_NVME_AUTH) += nvme-auth.o +obj-$(CONFIG_NVME_KEYRING) += nvme-keyring.o -nvme-common-$(CONFIG_NVME_AUTH) += auth.o -nvme-common-$(CONFIG_NVME_KEYRING) += keyring.o +nvme-auth-y += auth.o +nvme-keyring-y += keyring.o diff --git a/drivers/nvme/common/keyring.c b/drivers/nvme/common/keyring.c index f8d9a208397b..46d7a537dbc2 100644 --- a/drivers/nvme/common/keyring.c +++ b/drivers/nvme/common/keyring.c @@ -180,3 +180,5 @@ void nvme_keyring_exit(void) key_put(nvme_keyring); } EXPORT_SYMBOL_GPL(nvme_keyring_exit); + +MODULE_LICENSE("GPL v2"); diff --git a/drivers/nvme/host/Kconfig b/drivers/nvme/host/Kconfig index 48f7d72de5e9..8fe2dd619e80 100644 --- a/drivers/nvme/host/Kconfig +++ b/drivers/nvme/host/Kconfig @@ -95,7 +95,6 @@ config NVME_TCP config NVME_TCP_TLS bool "NVMe over Fabrics TCP TLS encryption support" depends on NVME_TCP - select NVME_COMMON select NVME_KEYRING select NET_HANDSHAKE select KEYS @@ -110,7 +109,6 @@ config NVME_TCP_TLS config NVME_HOST_AUTH bool "NVM Express over Fabrics In-Band Authentication" depends on NVME_CORE - select NVME_COMMON select NVME_AUTH help This provides support for NVMe over Fabrics In-Band Authentication. diff --git a/drivers/nvme/target/Kconfig b/drivers/nvme/target/Kconfig index fa479c9f5c3d..31633da9427c 100644 --- a/drivers/nvme/target/Kconfig +++ b/drivers/nvme/target/Kconfig @@ -87,7 +87,6 @@ config NVME_TARGET_TCP config NVME_TARGET_TCP_TLS bool "NVMe over Fabrics TCP target TLS encryption support" depends on NVME_TARGET_TCP - select NVME_COMMON select NVME_KEYRING select NET_HANDSHAKE select KEYS @@ -102,7 +101,6 @@ config NVME_TARGET_TCP_TLS config NVME_TARGET_AUTH bool "NVMe over Fabrics In-band Authentication support" depends on NVME_TARGET - select NVME_COMMON select NVME_AUTH help This enables support for NVMe over Fabrics In-band Authentication diff --git a/include/linux/nvme-keyring.h b/include/linux/nvme-keyring.h index 4efea9dd967c..6cc0696625f3 100644 --- a/include/linux/nvme-keyring.h +++ b/include/linux/nvme-keyring.h @@ -6,7 +6,7 @@ #ifndef _NVME_KEYRING_H #define _NVME_KEYRING_H -#ifdef CONFIG_NVME_KEYRING +#if IS_ENABLED(CONFIG_NVME_KEYRING) key_serial_t nvme_tls_psk_default(struct key *keyring, const char *hostnqn, const char *subnqn); From 706add13676da7ad213b65e92b94af5efc8c4131 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Thu, 26 Oct 2023 15:08:04 +0200 Subject: [PATCH 14/14] nvme: keyring: fix conditional compilation The keyring and auth functions can be called from both the host and the target side and are controlled by Kconfig options for each of the combinations, but the declarations are controlled by #ifdef checks on the shared Kconfig symbols. This leads to link failures in combinations where one of the frontends is built-in and the other one is a module, and the keyring code ends up in a module that is not reachable from the builtin code: ld: drivers/nvme/host/core.o: in function `nvme_core_exit': core.c:(.exit.text+0x4): undefined reference to `nvme_keyring_exit' ld: drivers/nvme/host/core.o: in function `nvme_core_init': core.c:(.init.text+0x94): undefined reference to `nvme_keyring_init ld: drivers/nvme/host/tcp.o: in function `nvme_tcp_setup_ctrl': tcp.c:(.text+0x4c18): undefined reference to `nvme_tls_psk_default' Address this by moving nvme_keyring_init()/nvme_keyring_exit() into module init/exit functions for the keyring module. Fixes: be8e82caa6859 ("nvme-tcp: enable TLS handshake upcall") Signed-off-by: Hannes Reinecke Cc: Arnd Bergmann Reviewed-by: Christoph Hellwig Signed-off-by: Keith Busch --- drivers/nvme/common/keyring.c | 9 +++++---- drivers/nvme/host/core.c | 9 +-------- include/linux/nvme-keyring.h | 8 -------- 3 files changed, 6 insertions(+), 20 deletions(-) diff --git a/drivers/nvme/common/keyring.c b/drivers/nvme/common/keyring.c index 46d7a537dbc2..ee341b83eeba 100644 --- a/drivers/nvme/common/keyring.c +++ b/drivers/nvme/common/keyring.c @@ -151,7 +151,7 @@ key_serial_t nvme_tls_psk_default(struct key *keyring, } EXPORT_SYMBOL_GPL(nvme_tls_psk_default); -int nvme_keyring_init(void) +static int __init nvme_keyring_init(void) { int err; @@ -171,14 +171,15 @@ int nvme_keyring_init(void) } return 0; } -EXPORT_SYMBOL_GPL(nvme_keyring_init); -void nvme_keyring_exit(void) +static void __exit nvme_keyring_exit(void) { unregister_key_type(&nvme_tls_psk_key_type); key_revoke(nvme_keyring); key_put(nvme_keyring); } -EXPORT_SYMBOL_GPL(nvme_keyring_exit); MODULE_LICENSE("GPL v2"); +MODULE_AUTHOR("Hannes Reinecke "); +module_init(nvme_keyring_init); +module_exit(nvme_keyring_exit); diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 75a1b58a7a43..88b54cdcbd68 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -25,7 +25,6 @@ #include "nvme.h" #include "fabrics.h" #include -#include #define CREATE_TRACE_POINTS #include "trace.h" @@ -4737,16 +4736,11 @@ static int __init nvme_core_init(void) result = PTR_ERR(nvme_ns_chr_class); goto unregister_generic_ns; } - result = nvme_keyring_init(); - if (result) - goto destroy_ns_chr; result = nvme_init_auth(); if (result) - goto keyring_exit; + goto destroy_ns_chr; return 0; -keyring_exit: - nvme_keyring_exit(); destroy_ns_chr: class_destroy(nvme_ns_chr_class); unregister_generic_ns: @@ -4770,7 +4764,6 @@ static int __init nvme_core_init(void) static void __exit nvme_core_exit(void) { nvme_exit_auth(); - nvme_keyring_exit(); class_destroy(nvme_ns_chr_class); class_destroy(nvme_subsys_class); class_destroy(nvme_class); diff --git a/include/linux/nvme-keyring.h b/include/linux/nvme-keyring.h index 6cc0696625f3..e10333d78dbb 100644 --- a/include/linux/nvme-keyring.h +++ b/include/linux/nvme-keyring.h @@ -12,8 +12,6 @@ key_serial_t nvme_tls_psk_default(struct key *keyring, const char *hostnqn, const char *subnqn); key_serial_t nvme_keyring_id(void); -int nvme_keyring_init(void); -void nvme_keyring_exit(void); #else @@ -26,11 +24,5 @@ static inline key_serial_t nvme_keyring_id(void) { return 0; } -static inline int nvme_keyring_init(void) -{ - return 0; -} -static inline void nvme_keyring_exit(void) {} - #endif /* !CONFIG_NVME_KEYRING */ #endif /* _NVME_KEYRING_H */