|
| 1 | +net/sched: act_api: fix possible infinite loop in tcf_idr_check_alloc() |
| 2 | + |
| 3 | +jira LE-3201 |
| 4 | +cve CVE-2024-40995 |
| 5 | +Rebuild_History Non-Buildable kernel-rt-4.18.0-553.22.1.rt7.363.el8_10 |
| 6 | +commit-author David Ruth <druth@chromium.org> |
| 7 | +commit d864319871b05fadd153e0aede4811ca7008f5d6 |
| 8 | +Empty-Commit: Cherry-Pick Conflicts during history rebuild. |
| 9 | +Will be included in final tarball splat. Ref for failed cherry-pick at: |
| 10 | +ciq/ciq_backports/kernel-rt-4.18.0-553.22.1.rt7.363.el8_10/d8643198.failed |
| 11 | + |
| 12 | +syzbot found hanging tasks waiting on rtnl_lock [1] |
| 13 | + |
| 14 | +A reproducer is available in the syzbot bug. |
| 15 | + |
| 16 | +When a request to add multiple actions with the same index is sent, the |
| 17 | +second request will block forever on the first request. This holds |
| 18 | +rtnl_lock, and causes tasks to hang. |
| 19 | + |
| 20 | +Return -EAGAIN to prevent infinite looping, while keeping documented |
| 21 | +behavior. |
| 22 | + |
| 23 | +[1] |
| 24 | + |
| 25 | +INFO: task kworker/1:0:5088 blocked for more than 143 seconds. |
| 26 | +Not tainted 6.9.0-rc4-syzkaller-00173-g3cdb45594619 #0 |
| 27 | +"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. |
| 28 | +task:kworker/1:0 state:D stack:23744 pid:5088 tgid:5088 ppid:2 flags:0x00004000 |
| 29 | +Workqueue: events_power_efficient reg_check_chans_work |
| 30 | +Call Trace: |
| 31 | +<TASK> |
| 32 | +context_switch kernel/sched/core.c:5409 [inline] |
| 33 | +__schedule+0xf15/0x5d00 kernel/sched/core.c:6746 |
| 34 | +__schedule_loop kernel/sched/core.c:6823 [inline] |
| 35 | +schedule+0xe7/0x350 kernel/sched/core.c:6838 |
| 36 | +schedule_preempt_disabled+0x13/0x30 kernel/sched/core.c:6895 |
| 37 | +__mutex_lock_common kernel/locking/mutex.c:684 [inline] |
| 38 | +__mutex_lock+0x5b8/0x9c0 kernel/locking/mutex.c:752 |
| 39 | +wiphy_lock include/net/cfg80211.h:5953 [inline] |
| 40 | +reg_leave_invalid_chans net/wireless/reg.c:2466 [inline] |
| 41 | +reg_check_chans_work+0x10a/0x10e0 net/wireless/reg.c:2481 |
| 42 | + |
| 43 | +Fixes: 0190c1d452a9 ("net: sched: atomically check-allocate action") |
| 44 | + Reported-by: syzbot+b87c222546179f4513a7@syzkaller.appspotmail.com |
| 45 | +Closes: https://syzkaller.appspot.com/bug?extid=b87c222546179f4513a7 |
| 46 | + Signed-off-by: David Ruth <druth@chromium.org> |
| 47 | + Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com> |
| 48 | +Link: https://lore.kernel.org/r/20240614190326.1349786-1-druth@chromium.org |
| 49 | + Signed-off-by: Paolo Abeni <pabeni@redhat.com> |
| 50 | + |
| 51 | +(cherry picked from commit d864319871b05fadd153e0aede4811ca7008f5d6) |
| 52 | + Signed-off-by: Jonathan Maple <jmaple@ciq.com> |
| 53 | + |
| 54 | +# Conflicts: |
| 55 | +# net/sched/act_api.c |
| 56 | +diff --cc net/sched/act_api.c |
| 57 | +index becdf74c8501,2520708b06a1..000000000000 |
| 58 | +--- a/net/sched/act_api.c |
| 59 | ++++ b/net/sched/act_api.c |
| 60 | +@@@ -830,43 -827,60 +830,53 @@@ int tcf_idr_check_alloc(struct tc_actio |
| 61 | + struct tcf_idrinfo *idrinfo = tn->idrinfo; |
| 62 | + struct tc_action *p; |
| 63 | + int ret; |
| 64 | + - u32 max; |
| 65 | + |
| 66 | +++<<<<<<< HEAD |
| 67 | + +again: |
| 68 | + + mutex_lock(&idrinfo->lock); |
| 69 | + + if (*index) { |
| 70 | +++======= |
| 71 | ++ if (*index) { |
| 72 | ++ rcu_read_lock(); |
| 73 | +++>>>>>>> d864319871b0 (net/sched: act_api: fix possible infinite loop in tcf_idr_check_alloc()) |
| 74 | + p = idr_find(&idrinfo->action_idr, *index); |
| 75 | + - |
| 76 | + if (IS_ERR(p)) { |
| 77 | + /* This means that another process allocated |
| 78 | + * index but did not assign the pointer yet. |
| 79 | + */ |
| 80 | +++<<<<<<< HEAD |
| 81 | + + mutex_unlock(&idrinfo->lock); |
| 82 | + + goto again; |
| 83 | +++======= |
| 84 | ++ rcu_read_unlock(); |
| 85 | ++ return -EAGAIN; |
| 86 | +++>>>>>>> d864319871b0 (net/sched: act_api: fix possible infinite loop in tcf_idr_check_alloc()) |
| 87 | + } |
| 88 | + |
| 89 | + - if (!p) { |
| 90 | + - /* Empty slot, try to allocate it */ |
| 91 | + - max = *index; |
| 92 | + - rcu_read_unlock(); |
| 93 | + - goto new; |
| 94 | + - } |
| 95 | + - |
| 96 | + - if (!refcount_inc_not_zero(&p->tcfa_refcnt)) { |
| 97 | + - /* Action was deleted in parallel */ |
| 98 | + - rcu_read_unlock(); |
| 99 | + - return -EAGAIN; |
| 100 | + + if (p) { |
| 101 | + + refcount_inc(&p->tcfa_refcnt); |
| 102 | + + if (bind) |
| 103 | + + atomic_inc(&p->tcfa_bindcnt); |
| 104 | + + *a = p; |
| 105 | + + ret = 1; |
| 106 | + + } else { |
| 107 | + + *a = NULL; |
| 108 | + + ret = idr_alloc_u32(&idrinfo->action_idr, NULL, index, |
| 109 | + + *index, GFP_KERNEL); |
| 110 | + + if (!ret) |
| 111 | + + idr_replace(&idrinfo->action_idr, |
| 112 | + + ERR_PTR(-EBUSY), *index); |
| 113 | + } |
| 114 | + - |
| 115 | + - if (bind) |
| 116 | + - atomic_inc(&p->tcfa_bindcnt); |
| 117 | + - *a = p; |
| 118 | + - |
| 119 | + - rcu_read_unlock(); |
| 120 | + - |
| 121 | + - return 1; |
| 122 | + } else { |
| 123 | + - /* Find a slot */ |
| 124 | + *index = 1; |
| 125 | + - max = UINT_MAX; |
| 126 | + + *a = NULL; |
| 127 | + + ret = idr_alloc_u32(&idrinfo->action_idr, NULL, index, |
| 128 | + + UINT_MAX, GFP_KERNEL); |
| 129 | + + if (!ret) |
| 130 | + + idr_replace(&idrinfo->action_idr, ERR_PTR(-EBUSY), |
| 131 | + + *index); |
| 132 | + } |
| 133 | + - |
| 134 | + -new: |
| 135 | + - *a = NULL; |
| 136 | + - |
| 137 | + - mutex_lock(&idrinfo->lock); |
| 138 | + - ret = idr_alloc_u32(&idrinfo->action_idr, ERR_PTR(-EBUSY), index, max, |
| 139 | + - GFP_KERNEL); |
| 140 | + mutex_unlock(&idrinfo->lock); |
| 141 | + - |
| 142 | + - /* N binds raced for action allocation, |
| 143 | + - * retry for all the ones that failed. |
| 144 | + - */ |
| 145 | + - if (ret == -ENOSPC && *index == max) |
| 146 | + - ret = -EAGAIN; |
| 147 | + - |
| 148 | + return ret; |
| 149 | + } |
| 150 | + EXPORT_SYMBOL(tcf_idr_check_alloc); |
| 151 | +* Unmerged path net/sched/act_api.c |
0 commit comments