Bug 275001 - if_wg: Missing radix unlock can cause deadlock
Summary: if_wg: Missing radix unlock can cause deadlock
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-net (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-11-10 02:52 UTC by Aaron LI
Modified: 2024-08-29 15:32 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Aaron LI 2023-11-10 02:52:45 UTC
In file 'sys/dev/wg/if_wg.c' function 'wg_aip_add()', the error path of returning ENOMEM when (node == NULL) is forgetting to unlock the radix tree, and thus may lead to a deadlock.

Below is the patch, please review.

--- if_wg.c.orig	2023-10-12 09:06:16.983637264 +0800
+++ if_wg.c	2023-10-21 15:29:47.928807521 +0800
@@ -562,7 +562,7 @@
 		node = root->rnh_lookup(&aip->a_addr, &aip->a_mask, &root->rh);
 	if (!node) {
 		free(aip, M_WG);
-		return (ENOMEM);
+		ret = ENOMEM;
 	} else if (node != aip->a_nodes) {
 		free(aip, M_WG);
 		aip = (struct wg_aip *)node;
Comment 1 commit-hook freebsd_committer freebsd_triage 2023-11-11 14:45:19 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=dcc4d2939f789a6d1f272ffeab2068ba2b7525ea

commit dcc4d2939f789a6d1f272ffeab2068ba2b7525ea
Author:     Aaron LI <aly@aaronly.me>
AuthorDate: 2023-11-11 13:13:08 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2023-11-11 13:13:08 +0000

    if_wg: Missing radix unlock can cause deadlock

    In function 'wg_aip_add()', the error path of returning ENOMEM when
    (node == NULL) is forgetting to unlock the radix tree, and thus may lead
    to a deadlock.

    PR:             275001
    Reviewed by:    kp
    MFC after:      1 week

 sys/dev/wg/if_wg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 2 Kristof Provost freebsd_committer freebsd_triage 2023-11-11 14:54:37 UTC
Thanks for the fix. I'll merge that back to 14 and 13 in a week or so and then close this bug.
Comment 3 Aaron LI 2023-11-12 04:07:31 UTC
(In reply to Kristof Provost from comment #2)

Thank you.
Comment 4 commit-hook freebsd_committer freebsd_triage 2023-11-19 19:30:36 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=9ed67f1307f40b3173ea3b3889c6323f9ef4a58f

commit 9ed67f1307f40b3173ea3b3889c6323f9ef4a58f
Author:     Aaron LI <aly@aaronly.me>
AuthorDate: 2023-11-11 13:13:08 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2023-11-18 09:46:21 +0000

    if_wg: Missing radix unlock can cause deadlock

    In function 'wg_aip_add()', the error path of returning ENOMEM when
    (node == NULL) is forgetting to unlock the radix tree, and thus may lead
    to a deadlock.

    PR:             275001
    Reviewed by:    kp
    MFC after:      1 week

    (cherry picked from commit dcc4d2939f789a6d1f272ffeab2068ba2b7525ea)

 sys/dev/wg/if_wg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 5 commit-hook freebsd_committer freebsd_triage 2023-11-19 19:30:41 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=16c65f7ae15e6ca8b36fc546e038c59270c87ccc

commit 16c65f7ae15e6ca8b36fc546e038c59270c87ccc
Author:     Aaron LI <aly@aaronly.me>
AuthorDate: 2023-11-11 13:13:08 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2023-11-19 19:29:35 +0000

    if_wg: Missing radix unlock can cause deadlock

    In function 'wg_aip_add()', the error path of returning ENOMEM when
    (node == NULL) is forgetting to unlock the radix tree, and thus may lead
    to a deadlock.

    PR:             275001
    Reviewed by:    kp
    MFC after:      1 week

    (cherry picked from commit dcc4d2939f789a6d1f272ffeab2068ba2b7525ea)

 sys/dev/wg/if_wg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 6 Mark Johnston freebsd_committer freebsd_triage 2023-11-24 15:36:10 UTC
Do we want an EN for this bug?  I'm not sure how easy it is to trigger in practice.