[PATCH OLK-6.6 0/4] nbd: replace socks pointer array with xarray to eliminate queue freeze
Hi, Commit b98e762e3d ("nbd: freeze the queue while we're adding connections") introduced blk_mq_freeze_queue() in the add-socket path to prevent use-after-free when krealloc() relocates the config->socks array while I/O is in flight. However, freezing the queue on every connection setup introduces significant latency when establishing a large number of connections. This series eliminates the queue freeze by replacing the krealloc-based struct nbd_sock **socks array with a struct xarray. The xarray provides RCU-safe pointer publishing: each nbd_sock is fully initialized before being stored via xa_store(), and concurrent readers access individual entries through xa_load() without ever holding a reference to the array itself. This removes the possibility of UAF on array reallocation, making the queue freeze unnecessary. The following test was performed with 256 connections on a local nbd-server: nbd-server -M 256 -C /etc/nbd-server/config time nbd-client 127.0.0.1 10809 /dev/nbd0 -N myexport -C 256 Before: real 4.510s user 0.004s sys 0.038s After: real 0.263s user 0.009s sys 0.032s Connection setup time is reduced by ~94%. Long Li (4): nbd: simplify find_fallback() by removing redundant logic nbd: replace socks pointer array with xarray nbd: remove redundant num_connections boundary checks nbd: remove queue freeze in nbd_add_socket drivers/block/nbd.c | 200 +++++++++++++++++++++++--------------------- 1 file changed, 106 insertions(+), 94 deletions(-) -- 2.52.0
hulk inclusion category: feature bugzilla: https://atomgit.com/openeuler/kernel/issues/9153 ---------------------------------------- The second conditional checking nsock->fallback_index validity is the logical inverse of the first, so drop it and let execution fall through naturally. Consolidate the two identical dev_err_ratelimited() + return paths into a single no_fallback label to reduce duplication. Signed-off-by: Long Li <leo.lilong@huawei.com> --- drivers/block/nbd.c | 37 ++++++++++++++----------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 9d367c8b7a2d..3b0e2e97acf9 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -952,40 +952,31 @@ static int find_fallback(struct nbd_device *nbd, int index) int new_index = -1; struct nbd_sock *nsock = config->socks[index]; int fallback = nsock->fallback_index; + int i; if (test_bit(NBD_RT_DISCONNECTED, &config->runtime_flags)) return new_index; - if (config->num_connections <= 1) { - dev_err_ratelimited(disk_to_dev(nbd->disk), - "Dead connection, failed to find a fallback\n"); - return new_index; - } + if (config->num_connections <= 1) + goto no_fallback; if (fallback >= 0 && fallback < config->num_connections && !config->socks[fallback]->dead) return fallback; - if (nsock->fallback_index < 0 || - nsock->fallback_index >= config->num_connections || - config->socks[nsock->fallback_index]->dead) { - int i; - for (i = 0; i < config->num_connections; i++) { - if (i == index) - continue; - if (!config->socks[i]->dead) { - new_index = i; - break; - } - } - nsock->fallback_index = new_index; - if (new_index < 0) { - dev_err_ratelimited(disk_to_dev(nbd->disk), - "Dead connection, failed to find a fallback\n"); - return new_index; + for (i = 0; i < config->num_connections; i++) { + if (i != index && !config->socks[i]->dead) { + new_index = i; + break; } } - new_index = nsock->fallback_index; + nsock->fallback_index = new_index; + if (new_index >= 0) + return new_index; + +no_fallback: + dev_err_ratelimited(disk_to_dev(nbd->disk), + "Dead connection, failed to find a fallback\n"); return new_index; } -- 2.52.0
hulk inclusion category: feature bugzilla: https://atomgit.com/openeuler/kernel/issues/9153 ---------------------------------------- Replace the krealloc-based struct nbd_sock **socks array with struct xarray socks. Each nbd sock is fully initialized before being stored into the xarray via xa_store(), ensuring concurrent readers calling xa_load() never observe a partially initialized socket. Convert all array index accesses to xa_load() and open-coded for-loops to xa_for_each(). Signed-off-by: Long Li <leo.lilong@huawei.com> --- drivers/block/nbd.c | 155 +++++++++++++++++++++++++++----------------- 1 file changed, 96 insertions(+), 59 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 3b0e2e97acf9..e108ec390ad2 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -38,6 +38,7 @@ #include <linux/types.h> #include <linux/debugfs.h> #include <linux/blk-mq.h> +#include <linux/xarray.h> #include <linux/uaccess.h> #include <asm/types.h> @@ -93,7 +94,7 @@ struct nbd_config { unsigned long runtime_flags; u64 dead_conn_timeout; - struct nbd_sock **socks; + struct xarray socks; int num_connections; atomic_t live_connections; wait_queue_head_t conn_wait; @@ -375,15 +376,15 @@ static void nbd_complete_rq(struct request *req) static void sock_shutdown(struct nbd_device *nbd) { struct nbd_config *config = nbd->config; - int i; + struct nbd_sock *nsock; + unsigned long i; if (config->num_connections == 0) return; if (test_and_set_bit(NBD_RT_DISCONNECTED, &config->runtime_flags)) return; - for (i = 0; i < config->num_connections; i++) { - struct nbd_sock *nsock = config->socks[i]; + xa_for_each(&config->socks, i, nsock) { mutex_lock(&nsock->tx_lock); nbd_mark_nsock_dead(nbd, nsock, 0); mutex_unlock(&nsock->tx_lock); @@ -428,6 +429,7 @@ static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req) struct nbd_cmd *cmd = blk_mq_rq_to_pdu(req); struct nbd_device *nbd = cmd->nbd; struct nbd_config *config; + struct nbd_sock *nsock; if (!mutex_trylock(&cmd->lock)) return BLK_EH_RESET_TIMER; @@ -457,10 +459,9 @@ static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req) * connection is configured, the submit path will wait util * a new connection is reconfigured or util dead timeout. */ - if (config->socks) { - if (cmd->index < config->num_connections) { - struct nbd_sock *nsock = - config->socks[cmd->index]; + if (!xa_empty(&config->socks)) { + nsock = xa_load(&config->socks, cmd->index); + if (nsock) { mutex_lock(&nsock->tx_lock); /* We can have multiple outstanding requests, so * we don't want to mark the nsock dead if we've @@ -484,22 +485,24 @@ static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req) * Userspace sets timeout=0 to disable socket disconnection, * so just warn and reset the timer. */ - struct nbd_sock *nsock = config->socks[cmd->index]; cmd->retries++; dev_info(nbd_to_dev(nbd), "Possible stuck request %p: control (%s@%llu,%uB). Runtime %u seconds\n", req, nbdcmd_to_ascii(req_to_nbd_cmd_type(req)), (unsigned long long)blk_rq_pos(req) << 9, blk_rq_bytes(req), (req->timeout / HZ) * cmd->retries); - mutex_lock(&nsock->tx_lock); - if (cmd->cookie != nsock->cookie) { - nbd_requeue_cmd(cmd); + nsock = xa_load(&config->socks, cmd->index); + if (nsock) { + mutex_lock(&nsock->tx_lock); + if (cmd->cookie != nsock->cookie) { + nbd_requeue_cmd(cmd); + mutex_unlock(&nsock->tx_lock); + mutex_unlock(&cmd->lock); + nbd_config_put(nbd); + return BLK_EH_DONE; + } mutex_unlock(&nsock->tx_lock); - mutex_unlock(&cmd->lock); - nbd_config_put(nbd); - return BLK_EH_DONE; } - mutex_unlock(&nsock->tx_lock); mutex_unlock(&cmd->lock); nbd_config_put(nbd); return BLK_EH_RESET_TIMER; @@ -566,8 +569,16 @@ static int sock_xmit(struct nbd_device *nbd, int index, int send, struct iov_iter *iter, int msg_flags, int *sent) { struct nbd_config *config = nbd->config; - struct socket *sock = config->socks[index]->sock; + struct nbd_sock *nsock; + struct socket *sock; + nsock = xa_load(&config->socks, index); + if (unlikely(!nsock)) { + dev_err_ratelimited(disk_to_dev(nbd->disk), + "Attempted xmit on invalid socket\n"); + return -EINVAL; + } + sock = nsock->sock; return __sock_xmit(nbd, sock, send, iter, msg_flags, sent); } @@ -588,7 +599,7 @@ static int nbd_send_cmd(struct nbd_device *nbd, struct nbd_cmd *cmd, int index) { struct request *req = blk_mq_rq_from_pdu(cmd); struct nbd_config *config = nbd->config; - struct nbd_sock *nsock = config->socks[index]; + struct nbd_sock *nsock; int result; struct nbd_request request = {.magic = htonl(NBD_REQUEST_MAGIC)}; struct kvec iov = {.iov_base = &request, .iov_len = sizeof(request)}; @@ -598,7 +609,14 @@ static int nbd_send_cmd(struct nbd_device *nbd, struct nbd_cmd *cmd, int index) u64 handle; u32 type; u32 nbd_cmd_flags = 0; - int sent = nsock->sent, skip = 0; + int sent, skip = 0; + + nsock = xa_load(&config->socks, index); + if (unlikely(!nsock)) { + dev_err_ratelimited(disk_to_dev(nbd->disk), + "Attempted send on invalid socket\n"); + return BLK_STS_IOERR; + } lockdep_assert_held(&cmd->lock); lockdep_assert_held(&nsock->tx_lock); @@ -623,6 +641,7 @@ static int nbd_send_cmd(struct nbd_device *nbd, struct nbd_cmd *cmd, int index) * request struct, so just go and send the rest of the pages in the * request. */ + sent = nsock->sent; if (sent) { if (sent >= sizeof(request)) { skip = sent - sizeof(request); @@ -950,9 +969,10 @@ static int find_fallback(struct nbd_device *nbd, int index) { struct nbd_config *config = nbd->config; int new_index = -1; - struct nbd_sock *nsock = config->socks[index]; - int fallback = nsock->fallback_index; - int i; + struct nbd_sock *nsock; + struct nbd_sock *fallback_nsock; + unsigned long i; + int fallback; if (test_bit(NBD_RT_DISCONNECTED, &config->runtime_flags)) return new_index; @@ -960,12 +980,19 @@ static int find_fallback(struct nbd_device *nbd, int index) if (config->num_connections <= 1) goto no_fallback; - if (fallback >= 0 && fallback < config->num_connections && - !config->socks[fallback]->dead) - return fallback; + nsock = xa_load(&config->socks, index); + if (unlikely(!nsock)) + goto no_fallback; - for (i = 0; i < config->num_connections; i++) { - if (i != index && !config->socks[i]->dead) { + fallback = nsock->fallback_index; + if (fallback >= 0 && fallback < config->num_connections) { + fallback_nsock = xa_load(&config->socks, fallback); + if (fallback_nsock && !fallback_nsock->dead) + return fallback; + } + + xa_for_each(&config->socks, i, fallback_nsock) { + if (i != index && !fallback_nsock->dead) { new_index = i; break; } @@ -1021,7 +1048,14 @@ static blk_status_t nbd_handle_cmd(struct nbd_cmd *cmd, int index) } cmd->status = BLK_STS_OK; again: - nsock = config->socks[index]; + nsock = xa_load(&config->socks, index); + if (unlikely(!nsock)) { + dev_err_ratelimited(disk_to_dev(nbd->disk), + "Attempted send on invalid socket\n"); + nbd_config_put(nbd); + return BLK_STS_IOERR; + } + mutex_lock(&nsock->tx_lock); if (nsock->dead) { int old_index = index; @@ -1142,8 +1176,8 @@ static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg, { struct nbd_config *config = nbd->config; struct socket *sock; - struct nbd_sock **socks; struct nbd_sock *nsock; + unsigned int index; int err; /* Arg will be cast to int, check it to avoid overflow */ @@ -1178,16 +1212,6 @@ static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg, goto put_socket; } - socks = krealloc(config->socks, (config->num_connections + 1) * - sizeof(struct nbd_sock *), GFP_KERNEL); - if (!socks) { - kfree(nsock); - err = -ENOMEM; - goto put_socket; - } - - config->socks = socks; - nsock->fallback_index = -1; nsock->dead = false; mutex_init(&nsock->tx_lock); @@ -1195,7 +1219,14 @@ static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg, nsock->pending = NULL; nsock->sent = 0; nsock->cookie = 0; - socks[config->num_connections++] = nsock; + + err = xa_alloc(&config->socks, &index, nsock, xa_limit_32b, GFP_KERNEL); + if (err < 0) { + kfree(nsock); + goto put_socket; + } + + config->num_connections++; atomic_inc(&config->live_connections); blk_mq_unfreeze_queue(nbd->disk->queue); @@ -1212,7 +1243,8 @@ static int nbd_reconnect_socket(struct nbd_device *nbd, unsigned long arg) struct nbd_config *config = nbd->config; struct socket *sock, *old; struct recv_thread_args *args; - int i; + struct nbd_sock *nsock; + unsigned long i; int err; sock = nbd_get_socket(nbd, arg, &err); @@ -1225,9 +1257,7 @@ static int nbd_reconnect_socket(struct nbd_device *nbd, unsigned long arg) return -ENOMEM; } - for (i = 0; i < config->num_connections; i++) { - struct nbd_sock *nsock = config->socks[i]; - + xa_for_each(&config->socks, i, nsock) { if (!nsock->dead) continue; @@ -1302,10 +1332,11 @@ static void send_disconnects(struct nbd_device *nbd) }; struct kvec iov = {.iov_base = &request, .iov_len = sizeof(request)}; struct iov_iter from; - int i, ret; + struct nbd_sock *nsock; + unsigned long i; + int ret; - for (i = 0; i < config->num_connections; i++) { - struct nbd_sock *nsock = config->socks[i]; + xa_for_each(&config->socks, i, nsock) { iov_iter_kvec(&from, ITER_SOURCE, &iov, 1, sizeof(request)); mutex_lock(&nsock->tx_lock); @@ -1340,6 +1371,9 @@ static void nbd_config_put(struct nbd_device *nbd) if (refcount_dec_and_mutex_lock(&nbd->config_refs, &nbd->config_lock)) { struct nbd_config *config = nbd->config; + struct nbd_sock *nsock; + unsigned long i; + nbd_dev_dbg_close(nbd); invalidate_disk(nbd->disk); if (nbd->config->bytesize) @@ -1355,14 +1389,15 @@ static void nbd_config_put(struct nbd_device *nbd) nbd->backend = NULL; } nbd_clear_sock(nbd); + if (config->num_connections) { - int i; - for (i = 0; i < config->num_connections; i++) { - sockfd_put(config->socks[i]->sock); - kfree(config->socks[i]); + xa_for_each(&config->socks, i, nsock) { + sockfd_put(nsock->sock); + kfree(nsock); } - kfree(config->socks); } + xa_destroy(&config->socks); + kfree(nbd->config); nbd->config = NULL; @@ -1380,11 +1415,13 @@ static int nbd_start_device(struct nbd_device *nbd) { struct nbd_config *config = nbd->config; int num_connections = config->num_connections; - int error = 0, i; + int error = 0; + unsigned long i; + struct nbd_sock *nsock; if (nbd->pid) return -EBUSY; - if (!config->socks) + if (xa_empty(&config->socks)) return -EINVAL; if (num_connections > 1 && !(config->flags & NBD_FLAG_CAN_MULTI_CONN)) { @@ -1405,7 +1442,7 @@ static int nbd_start_device(struct nbd_device *nbd) set_bit(NBD_RT_HAS_PID_FILE, &config->runtime_flags); nbd_dev_dbg_init(nbd); - for (i = 0; i < num_connections; i++) { + xa_for_each(&config->socks, i, nsock) { struct recv_thread_args *args; args = kzalloc(sizeof(*args), GFP_KERNEL); @@ -1423,15 +1460,14 @@ static int nbd_start_device(struct nbd_device *nbd) flush_workqueue(nbd->recv_workq); return -ENOMEM; } - sk_set_memalloc(config->socks[i]->sock->sk); + sk_set_memalloc(nsock->sock->sk); if (nbd->tag_set.timeout) - config->socks[i]->sock->sk->sk_sndtimeo = - nbd->tag_set.timeout; + nsock->sock->sk->sk_sndtimeo = nbd->tag_set.timeout; atomic_inc(&config->recv_threads); refcount_inc(&nbd->config_refs); INIT_WORK(&args->work, recv_work); args->nbd = nbd; - args->nsock = config->socks[i]; + args->nsock = nsock; args->index = i; queue_work(nbd->recv_workq, &args->work); } @@ -1586,6 +1622,7 @@ static int nbd_alloc_and_init_config(struct nbd_device *nbd) return -ENOMEM; } + xa_init_flags(&config->socks, XA_FLAGS_ALLOC); atomic_set(&config->recv_threads, 0); init_waitqueue_head(&config->recv_wq); init_waitqueue_head(&config->conn_wait); -- 2.52.0
hulk inclusion category: feature bugzilla: https://atomgit.com/openeuler/kernel/issues/9153 ---------------------------------------- Now that config->socks uses xarray instead of a plain array, explicit bounds checking against num_connections is no longer necessary. xa_load() returns NULL for any out-of-range or missing index, and xa_for_each() is a no-op on an empty xarray, making these guards redundant. Signed-off-by: Long Li <leo.lilong@huawei.com> --- drivers/block/nbd.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index e108ec390ad2..198655b6f756 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -985,7 +985,7 @@ static int find_fallback(struct nbd_device *nbd, int index) goto no_fallback; fallback = nsock->fallback_index; - if (fallback >= 0 && fallback < config->num_connections) { + if (fallback >= 0) { fallback_nsock = xa_load(&config->socks, fallback); if (fallback_nsock && !fallback_nsock->dead) return fallback; @@ -1040,12 +1040,6 @@ static blk_status_t nbd_handle_cmd(struct nbd_cmd *cmd, int index) return BLK_STS_IOERR; } - if (index >= config->num_connections) { - dev_err_ratelimited(disk_to_dev(nbd->disk), - "Attempted send on invalid socket\n"); - nbd_config_put(nbd); - return BLK_STS_IOERR; - } cmd->status = BLK_STS_OK; again: nsock = xa_load(&config->socks, index); @@ -1390,11 +1384,9 @@ static void nbd_config_put(struct nbd_device *nbd) } nbd_clear_sock(nbd); - if (config->num_connections) { - xa_for_each(&config->socks, i, nsock) { - sockfd_put(nsock->sock); - kfree(nsock); - } + xa_for_each(&config->socks, i, nsock) { + sockfd_put(nsock->sock); + kfree(nsock); } xa_destroy(&config->socks); -- 2.52.0
hulk inclusion category: feature bugzilla: https://atomgit.com/openeuler/kernel/issues/9153 ---------------------------------------- The queue freeze was originally needed to prevent concurrent requests from accessing config->socks while the backing array was being reallocated. Since config->socks is now an xarray, insertions are safe under RCU without freezing the queue. This significantly reduces connection setup time when using a large number of connections (-C 256): before: real 4.510s after: real 0.263s Signed-off-by: Long Li <leo.lilong@huawei.com> --- drivers/block/nbd.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 198655b6f756..f9b51b09730a 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1181,12 +1181,6 @@ static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg, if (!sock) return err; - /* - * We need to make sure we don't get any errant requests while we're - * reallocating the ->socks array. - */ - blk_mq_freeze_queue(nbd->disk->queue); - if (!netlink && !nbd->task_setup && !test_bit(NBD_RT_BOUND, &config->runtime_flags)) nbd->task_setup = current; @@ -1222,12 +1216,10 @@ static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg, config->num_connections++; atomic_inc(&config->live_connections); - blk_mq_unfreeze_queue(nbd->disk->queue); return 0; put_socket: - blk_mq_unfreeze_queue(nbd->disk->queue); sockfd_put(sock); return err; } -- 2.52.0
反馈： 您发送到kernel@openeuler.org的补丁/补丁集，已成功转换为PR！ PR链接地址： https://atomgit.com/openeuler/kernel/merge_requests/22443 邮件列表地址：https://mailweb.openeuler.org/archives/list/kernel@openeuler.org/message/6C3... FeedBack: The patch(es) which you have sent to kernel@openeuler.org mailing list has been converted to a pull request successfully! Pull request link: https://atomgit.com/openeuler/kernel/merge_requests/22443 Mailing list address: https://mailweb.openeuler.org/archives/list/kernel@openeuler.org/message/6C3...
participants (2)
-
Long Li -
patchwork bot