diff for duplicates of <20260607095730.443050343@linuxfoundation.org> diff --git a/a/1.txt b/N1/1.txt index ee4adf1..6857dd5 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,136 +1,99 @@ -6.18-stable review patch. If anyone has any objections, please let me know. +6.12-stable review patch. If anyone has any objections, please let me know. ------------------ -From: Chuck Lever <chuck.lever@oracle.com> +From: Jamal Hadi Salim <jhs@mojatatu.com> -[ Upstream commit cc993e0927ec8bd98ea33377ada03295fcda0f24 ] +[ Upstream commit eda0b7f203bb166c98d1418b204135bd566ac83b ] -nvmet_tcp_state_change(), a socket callback that runs in BH context, -can reach handshake_req_cancel() via nvmet_tcp_schedule_release_queue() -and tls_handshake_cancel(). handshake_req_cancel() acquires -hn->hn_lock with plain spin_lock(). If a process-context thread on -the same CPU holds hn->hn_lock when a softirq invokes the cancel path, -the lock attempt deadlocks. This is the only caller that invokes -tls_handshake_cancel() from BH context; every other consumer calls it -from process context. +This reverts commit ec8e0e3d7adef940cdf9475e2352c0680189d14e. -Deferring the cancel to process context in the NVMe target is not -straightforward: nvmet_tcp_schedule_release_queue() must call -tls_handshake_cancel() atomically with its state transition to -DISCONNECTING. If the cancel were deferred, the handshake completion -callback could fire in the window before the cancel runs, observe the -unexpected state, and return without dropping its kref on the queue. -Reworking that interlock is considerably more invasive than hardening -the handshake lock. Convert all hn->hn_lock acquisitions from -spin_lock/spin_unlock to spin_lock_bh/spin_unlock_bh so the lock is -never taken with softirqs enabled. +The original patch rejects any tree containing two netems when +either has duplication set, even when they sit on unrelated classes +of the same classful parent. That broke configurations that have +worked since netem was introduced. -Fixes: 675b453e0241 ("nvmet-tcp: enable TLS handshake upcall") -Signed-off-by: Chuck Lever <chuck.lever@oracle.com> -Reviewed-by: Hannes Reinecke <hare@kernel.org> -Link: https://patch.msgid.link/20260525-handshake-file-pin-v3-1-66c616906ead@oracle.com +The re-entrancy problem the original commit was trying to solve is +handled by later patch using tc_depth flag. + +Doing this revert will (re)expose the original bug with multiple +netem duplication. When this patch is backported make sure +and get the full series. + +Fixes: ec8e0e3d7ade ("net/sched: Restrict conditions for adding duplicating netems to qdisc tree") +Reported-by: Ji-Soo Chung <jschung2@proton.me> +Reported-by: Gerlinde <lrGerlinde@mailfence.com> +Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220774 +Reported-by: zyc zyc <zyc199902@zohomail.cn> +Closes: https://lore.kernel.org/all/19adda5a1e2.12410b78222774.9191120410578703463@zohomail.cn/ +Reported-by: Manas Ghandat <ghandatmanas@gmail.com> +Closes: https://lore.kernel.org/netdev/f69b2c8f-8325-4c2e-a011-6dbc089f30e4@gmail.com/ +Reviewed-by: Stephen Hemminger <stephen@networkplumber.org> +Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> +Link: https://patch.msgid.link/20260525122556.973584-3-jhs@mojatatu.com Signed-off-by: Paolo Abeni <pabeni@redhat.com> Signed-off-by: Sasha Levin <sashal@kernel.org> --- - net/handshake/netlink.c | 4 ++-- - net/handshake/request.c | 14 +++++++------- - net/handshake/tlshd.c | 2 ++ - 3 files changed, 11 insertions(+), 9 deletions(-) + net/sched/sch_netem.c | 40 ---------------------------------------- + 1 file changed, 40 deletions(-) -diff --git a/net/handshake/netlink.c b/net/handshake/netlink.c -index 7e46d130dce2cd..394e270cc505cb 100644 ---- a/net/handshake/netlink.c -+++ b/net/handshake/netlink.c -@@ -203,10 +203,10 @@ static void __net_exit handshake_net_exit(struct net *net) - * accepted and are in progress will be destroyed when - * the socket is closed. - */ -- spin_lock(&hn->hn_lock); -+ spin_lock_bh(&hn->hn_lock); - set_bit(HANDSHAKE_F_NET_DRAINING, &hn->hn_flags); - list_splice_init(&requests, &hn->hn_requests); -- spin_unlock(&hn->hn_lock); -+ spin_unlock_bh(&hn->hn_lock); - - while (!list_empty(&requests)) { - req = list_first_entry(&requests, struct handshake_req, hr_list); -diff --git a/net/handshake/request.c b/net/handshake/request.c -index 6b7e3e0bf3996e..654e55b141cded 100644 ---- a/net/handshake/request.c -+++ b/net/handshake/request.c -@@ -167,12 +167,12 @@ static bool remove_pending(struct handshake_net *hn, struct handshake_req *req) - { - bool ret = false; - -- spin_lock(&hn->hn_lock); -+ spin_lock_bh(&hn->hn_lock); - if (!list_empty(&req->hr_list)) { - __remove_pending_locked(hn, req); - ret = true; - } -- spin_unlock(&hn->hn_lock); -+ spin_unlock_bh(&hn->hn_lock); - - return ret; - } -@@ -182,7 +182,7 @@ struct handshake_req *handshake_req_next(struct handshake_net *hn, int class) - struct handshake_req *req, *pos; - - req = NULL; -- spin_lock(&hn->hn_lock); -+ spin_lock_bh(&hn->hn_lock); - list_for_each_entry(pos, &hn->hn_requests, hr_list) { - if (pos->hr_proto->hp_handler_class != class) - continue; -@@ -190,7 +190,7 @@ struct handshake_req *handshake_req_next(struct handshake_net *hn, int class) - req = pos; - break; - } -- spin_unlock(&hn->hn_lock); -+ spin_unlock_bh(&hn->hn_lock); - - return req; +diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c +index 498c18d7d9c39b..1fdebf2ab7ee46 100644 +--- a/net/sched/sch_netem.c ++++ b/net/sched/sch_netem.c +@@ -1005,41 +1005,6 @@ static int parse_attr(struct nlattr *tb[], int maxtype, struct nlattr *nla, + return 0; } -@@ -249,7 +249,7 @@ int handshake_req_submit(struct socket *sock, struct handshake_req *req, - if (READ_ONCE(hn->hn_pending) >= hn->hn_pending_max) - goto out_err; -- spin_lock(&hn->hn_lock); -+ spin_lock_bh(&hn->hn_lock); - ret = -EOPNOTSUPP; - if (test_bit(HANDSHAKE_F_NET_DRAINING, &hn->hn_flags)) - goto out_unlock; -@@ -258,7 +258,7 @@ int handshake_req_submit(struct socket *sock, struct handshake_req *req, - goto out_unlock; - if (!__add_pending_locked(hn, req)) - goto out_unlock; -- spin_unlock(&hn->hn_lock); -+ spin_unlock_bh(&hn->hn_lock); - - ret = handshake_genl_notify(net, req->hr_proto, flags); - if (ret) { -@@ -274,7 +274,7 @@ int handshake_req_submit(struct socket *sock, struct handshake_req *req, - return 0; +-static const struct Qdisc_class_ops netem_class_ops; +- +-static int check_netem_in_tree(struct Qdisc *sch, bool duplicates, +- struct netlink_ext_ack *extack) +-{ +- struct Qdisc *root, *q; +- unsigned int i; +- +- root = qdisc_root_sleeping(sch); +- +- if (sch != root && root->ops->cl_ops == &netem_class_ops) { +- if (duplicates || +- ((struct netem_sched_data *)qdisc_priv(root))->duplicate) +- goto err; +- } +- +- if (!qdisc_dev(root)) +- return 0; +- +- hash_for_each(qdisc_dev(root)->qdisc_hash, i, q, hash) { +- if (sch != q && q->ops->cl_ops == &netem_class_ops) { +- if (duplicates || +- ((struct netem_sched_data *)qdisc_priv(q))->duplicate) +- goto err; +- } +- } +- +- return 0; +- +-err: +- NL_SET_ERR_MSG(extack, +- "netem: cannot mix duplicating netems with other netems in tree"); +- return -EINVAL; +-} +- + /* Parse netlink message to set options */ + static int netem_change(struct Qdisc *sch, struct nlattr *opt, + struct netlink_ext_ack *extack) +@@ -1116,11 +1081,6 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt, + q->gap = qopt->gap; + q->counter = 0; + q->loss = qopt->loss; +- +- ret = check_netem_in_tree(sch, qopt->duplicate, extack); +- if (ret) +- goto unlock; +- + q->duplicate = qopt->duplicate; - out_unlock: -- spin_unlock(&hn->hn_lock); -+ spin_unlock_bh(&hn->hn_lock); - out_err: - /* Restore original destructor so socket teardown still runs on failure */ - req->hr_sk->sk_destruct = req->hr_odestruct; -diff --git a/net/handshake/tlshd.c b/net/handshake/tlshd.c -index 8f9532a15f43f9..af294c6cc71731 100644 ---- a/net/handshake/tlshd.c -+++ b/net/handshake/tlshd.c -@@ -425,6 +425,8 @@ EXPORT_SYMBOL(tls_server_hello_psk); - * Request cancellation races with request completion. To determine - * who won, callers examine the return value from this function. - * -+ * Context: May be called from process or softirq context. -+ * - * Return values: - * %true - Uncompleted handshake request was canceled - * %false - Handshake request already completed or not found + /* for compatibility with earlier versions. -- 2.53.0 diff --git a/a/content_digest b/N1/content_digest index 445cba2..1788c63 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,151 +1,118 @@ - "ref\020260607095727.528828913@linuxfoundation.org\0" + "ref\020260607095727.647295505@linuxfoundation.org\0" "From\0Greg Kroah-Hartman <gregkh@linuxfoundation.org>\0" - "Subject\0[PATCH 6.18 077/315] net/handshake: Use spin_lock_bh for hn_lock\0" - "Date\0Sun, 7 Jun 2026 11:57:44 +0200\0" + "Subject\0[PATCH 6.12 073/307] net/sched: Revert \"net/sched: Restrict conditions for adding duplicating netems to qdisc tree\"\0" + "Date\0Sun, 7 Jun 2026 11:57:50 +0200\0" "To\0stable@vger.kernel.org\0" "Cc\0Greg Kroah-Hartman <gregkh@linuxfoundation.org>" patches@lists.linux.dev - Chuck Lever <chuck.lever@oracle.com> - Hannes Reinecke <hare@kernel.org> + Ji-Soo Chung <jschung2@proton.me> + Gerlinde <lrGerlinde@mailfence.com> + zyc zyc <zyc199902@zohomail.cn> + Manas Ghandat <ghandatmanas@gmail.com> + Stephen Hemminger <stephen@networkplumber.org> + Jamal Hadi Salim <jhs@mojatatu.com> Paolo Abeni <pabeni@redhat.com> " Sasha Levin <sashal@kernel.org>\0" "\00:1\0" "b\0" - "6.18-stable review patch. If anyone has any objections, please let me know.\n" + "6.12-stable review patch. If anyone has any objections, please let me know.\n" "\n" "------------------\n" "\n" - "From: Chuck Lever <chuck.lever@oracle.com>\n" + "From: Jamal Hadi Salim <jhs@mojatatu.com>\n" "\n" - "[ Upstream commit cc993e0927ec8bd98ea33377ada03295fcda0f24 ]\n" + "[ Upstream commit eda0b7f203bb166c98d1418b204135bd566ac83b ]\n" "\n" - "nvmet_tcp_state_change(), a socket callback that runs in BH context,\n" - "can reach handshake_req_cancel() via nvmet_tcp_schedule_release_queue()\n" - "and tls_handshake_cancel(). handshake_req_cancel() acquires\n" - "hn->hn_lock with plain spin_lock(). If a process-context thread on\n" - "the same CPU holds hn->hn_lock when a softirq invokes the cancel path,\n" - "the lock attempt deadlocks. This is the only caller that invokes\n" - "tls_handshake_cancel() from BH context; every other consumer calls it\n" - "from process context.\n" + "This reverts commit ec8e0e3d7adef940cdf9475e2352c0680189d14e.\n" "\n" - "Deferring the cancel to process context in the NVMe target is not\n" - "straightforward: nvmet_tcp_schedule_release_queue() must call\n" - "tls_handshake_cancel() atomically with its state transition to\n" - "DISCONNECTING. If the cancel were deferred, the handshake completion\n" - "callback could fire in the window before the cancel runs, observe the\n" - "unexpected state, and return without dropping its kref on the queue.\n" - "Reworking that interlock is considerably more invasive than hardening\n" - "the handshake lock. Convert all hn->hn_lock acquisitions from\n" - "spin_lock/spin_unlock to spin_lock_bh/spin_unlock_bh so the lock is\n" - "never taken with softirqs enabled.\n" + "The original patch rejects any tree containing two netems when\n" + "either has duplication set, even when they sit on unrelated classes\n" + "of the same classful parent. That broke configurations that have\n" + "worked since netem was introduced.\n" "\n" - "Fixes: 675b453e0241 (\"nvmet-tcp: enable TLS handshake upcall\")\n" - "Signed-off-by: Chuck Lever <chuck.lever@oracle.com>\n" - "Reviewed-by: Hannes Reinecke <hare@kernel.org>\n" - "Link: https://patch.msgid.link/20260525-handshake-file-pin-v3-1-66c616906ead@oracle.com\n" + "The re-entrancy problem the original commit was trying to solve is\n" + "handled by later patch using tc_depth flag.\n" + "\n" + "Doing this revert will (re)expose the original bug with multiple\n" + "netem duplication. When this patch is backported make sure\n" + "and get the full series.\n" + "\n" + "Fixes: ec8e0e3d7ade (\"net/sched: Restrict conditions for adding duplicating netems to qdisc tree\")\n" + "Reported-by: Ji-Soo Chung <jschung2@proton.me>\n" + "Reported-by: Gerlinde <lrGerlinde@mailfence.com>\n" + "Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220774\n" + "Reported-by: zyc zyc <zyc199902@zohomail.cn>\n" + "Closes: https://lore.kernel.org/all/19adda5a1e2.12410b78222774.9191120410578703463@zohomail.cn/\n" + "Reported-by: Manas Ghandat <ghandatmanas@gmail.com>\n" + "Closes: https://lore.kernel.org/netdev/f69b2c8f-8325-4c2e-a011-6dbc089f30e4@gmail.com/\n" + "Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>\n" + "Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>\n" + "Link: https://patch.msgid.link/20260525122556.973584-3-jhs@mojatatu.com\n" "Signed-off-by: Paolo Abeni <pabeni@redhat.com>\n" "Signed-off-by: Sasha Levin <sashal@kernel.org>\n" "---\n" - " net/handshake/netlink.c | 4 ++--\n" - " net/handshake/request.c | 14 +++++++-------\n" - " net/handshake/tlshd.c | 2 ++\n" - " 3 files changed, 11 insertions(+), 9 deletions(-)\n" + " net/sched/sch_netem.c | 40 ----------------------------------------\n" + " 1 file changed, 40 deletions(-)\n" "\n" - "diff --git a/net/handshake/netlink.c b/net/handshake/netlink.c\n" - "index 7e46d130dce2cd..394e270cc505cb 100644\n" - "--- a/net/handshake/netlink.c\n" - "+++ b/net/handshake/netlink.c\n" - "@@ -203,10 +203,10 @@ static void __net_exit handshake_net_exit(struct net *net)\n" - " \t * accepted and are in progress will be destroyed when\n" - " \t * the socket is closed.\n" - " \t */\n" - "-\tspin_lock(&hn->hn_lock);\n" - "+\tspin_lock_bh(&hn->hn_lock);\n" - " \tset_bit(HANDSHAKE_F_NET_DRAINING, &hn->hn_flags);\n" - " \tlist_splice_init(&requests, &hn->hn_requests);\n" - "-\tspin_unlock(&hn->hn_lock);\n" - "+\tspin_unlock_bh(&hn->hn_lock);\n" - " \n" - " \twhile (!list_empty(&requests)) {\n" - " \t\treq = list_first_entry(&requests, struct handshake_req, hr_list);\n" - "diff --git a/net/handshake/request.c b/net/handshake/request.c\n" - "index 6b7e3e0bf3996e..654e55b141cded 100644\n" - "--- a/net/handshake/request.c\n" - "+++ b/net/handshake/request.c\n" - "@@ -167,12 +167,12 @@ static bool remove_pending(struct handshake_net *hn, struct handshake_req *req)\n" - " {\n" - " \tbool ret = false;\n" - " \n" - "-\tspin_lock(&hn->hn_lock);\n" - "+\tspin_lock_bh(&hn->hn_lock);\n" - " \tif (!list_empty(&req->hr_list)) {\n" - " \t\t__remove_pending_locked(hn, req);\n" - " \t\tret = true;\n" - " \t}\n" - "-\tspin_unlock(&hn->hn_lock);\n" - "+\tspin_unlock_bh(&hn->hn_lock);\n" - " \n" - " \treturn ret;\n" - " }\n" - "@@ -182,7 +182,7 @@ struct handshake_req *handshake_req_next(struct handshake_net *hn, int class)\n" - " \tstruct handshake_req *req, *pos;\n" - " \n" - " \treq = NULL;\n" - "-\tspin_lock(&hn->hn_lock);\n" - "+\tspin_lock_bh(&hn->hn_lock);\n" - " \tlist_for_each_entry(pos, &hn->hn_requests, hr_list) {\n" - " \t\tif (pos->hr_proto->hp_handler_class != class)\n" - " \t\t\tcontinue;\n" - "@@ -190,7 +190,7 @@ struct handshake_req *handshake_req_next(struct handshake_net *hn, int class)\n" - " \t\treq = pos;\n" - " \t\tbreak;\n" - " \t}\n" - "-\tspin_unlock(&hn->hn_lock);\n" - "+\tspin_unlock_bh(&hn->hn_lock);\n" - " \n" - " \treturn req;\n" + "diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c\n" + "index 498c18d7d9c39b..1fdebf2ab7ee46 100644\n" + "--- a/net/sched/sch_netem.c\n" + "+++ b/net/sched/sch_netem.c\n" + "@@ -1005,41 +1005,6 @@ static int parse_attr(struct nlattr *tb[], int maxtype, struct nlattr *nla,\n" + " \treturn 0;\n" " }\n" - "@@ -249,7 +249,7 @@ int handshake_req_submit(struct socket *sock, struct handshake_req *req,\n" - " \tif (READ_ONCE(hn->hn_pending) >= hn->hn_pending_max)\n" - " \t\tgoto out_err;\n" " \n" - "-\tspin_lock(&hn->hn_lock);\n" - "+\tspin_lock_bh(&hn->hn_lock);\n" - " \tret = -EOPNOTSUPP;\n" - " \tif (test_bit(HANDSHAKE_F_NET_DRAINING, &hn->hn_flags))\n" - " \t\tgoto out_unlock;\n" - "@@ -258,7 +258,7 @@ int handshake_req_submit(struct socket *sock, struct handshake_req *req,\n" - " \t\tgoto out_unlock;\n" - " \tif (!__add_pending_locked(hn, req))\n" - " \t\tgoto out_unlock;\n" - "-\tspin_unlock(&hn->hn_lock);\n" - "+\tspin_unlock_bh(&hn->hn_lock);\n" - " \n" - " \tret = handshake_genl_notify(net, req->hr_proto, flags);\n" - " \tif (ret) {\n" - "@@ -274,7 +274,7 @@ int handshake_req_submit(struct socket *sock, struct handshake_req *req,\n" - " \treturn 0;\n" + "-static const struct Qdisc_class_ops netem_class_ops;\n" + "-\n" + "-static int check_netem_in_tree(struct Qdisc *sch, bool duplicates,\n" + "-\t\t\t struct netlink_ext_ack *extack)\n" + "-{\n" + "-\tstruct Qdisc *root, *q;\n" + "-\tunsigned int i;\n" + "-\n" + "-\troot = qdisc_root_sleeping(sch);\n" + "-\n" + "-\tif (sch != root && root->ops->cl_ops == &netem_class_ops) {\n" + "-\t\tif (duplicates ||\n" + "-\t\t ((struct netem_sched_data *)qdisc_priv(root))->duplicate)\n" + "-\t\t\tgoto err;\n" + "-\t}\n" + "-\n" + "-\tif (!qdisc_dev(root))\n" + "-\t\treturn 0;\n" + "-\n" + "-\thash_for_each(qdisc_dev(root)->qdisc_hash, i, q, hash) {\n" + "-\t\tif (sch != q && q->ops->cl_ops == &netem_class_ops) {\n" + "-\t\t\tif (duplicates ||\n" + "-\t\t\t ((struct netem_sched_data *)qdisc_priv(q))->duplicate)\n" + "-\t\t\t\tgoto err;\n" + "-\t\t}\n" + "-\t}\n" + "-\n" + "-\treturn 0;\n" + "-\n" + "-err:\n" + "-\tNL_SET_ERR_MSG(extack,\n" + "-\t\t \"netem: cannot mix duplicating netems with other netems in tree\");\n" + "-\treturn -EINVAL;\n" + "-}\n" + "-\n" + " /* Parse netlink message to set options */\n" + " static int netem_change(struct Qdisc *sch, struct nlattr *opt,\n" + " \t\t\tstruct netlink_ext_ack *extack)\n" + "@@ -1116,11 +1081,6 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt,\n" + " \tq->gap = qopt->gap;\n" + " \tq->counter = 0;\n" + " \tq->loss = qopt->loss;\n" + "-\n" + "-\tret = check_netem_in_tree(sch, qopt->duplicate, extack);\n" + "-\tif (ret)\n" + "-\t\tgoto unlock;\n" + "-\n" + " \tq->duplicate = qopt->duplicate;\n" " \n" - " out_unlock:\n" - "-\tspin_unlock(&hn->hn_lock);\n" - "+\tspin_unlock_bh(&hn->hn_lock);\n" - " out_err:\n" - " \t/* Restore original destructor so socket teardown still runs on failure */\n" - " \treq->hr_sk->sk_destruct = req->hr_odestruct;\n" - "diff --git a/net/handshake/tlshd.c b/net/handshake/tlshd.c\n" - "index 8f9532a15f43f9..af294c6cc71731 100644\n" - "--- a/net/handshake/tlshd.c\n" - "+++ b/net/handshake/tlshd.c\n" - "@@ -425,6 +425,8 @@ EXPORT_SYMBOL(tls_server_hello_psk);\n" - " * Request cancellation races with request completion. To determine\n" - " * who won, callers examine the return value from this function.\n" - " *\n" - "+ * Context: May be called from process or softirq context.\n" - "+ *\n" - " * Return values:\n" - " * %true - Uncompleted handshake request was canceled\n" - " * %false - Handshake request already completed or not found\n" + " \t/* for compatibility with earlier versions.\n" "-- \n" 2.53.0 -e626ea172a59abb87a876d3128bb521f7db9e4ae9633230d55144b9e1ba2eb1e +3709b1bc4589cafed8937def7100468bfdd719303e136f73a730e994a528933b
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox