diff mbox series

[net,v5,1/2] net: sched: fix use-after-free in taprio_change()

Message ID 20240904120842.3426084-1-dmantipov@yandex.ru (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net,v5,1/2] net: sched: fix use-after-free in taprio_change() | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 16 this patch: 16
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 4 maintainers not CCed: xiyou.wangcong@gmail.com jiri@resnulli.us edumazet@google.com jhs@mojatatu.com
netdev/build_clang success Errors and warnings before: 16 this patch: 16
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 16 this patch: 16
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 9 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-09-07--06-00 (tests: 722)

Commit Message

Dmitry Antipov Sept. 4, 2024, 12:08 p.m. UTC
In 'taprio_change()', 'admin' pointer may become dangling due to sched
switch / removal caused by 'advance_sched()', and critical section
protected by 'q->current_entry_lock' is too small to prevent from such
a scenario (which causes use-after-free detected by KASAN). Fix this
by prefer 'rcu_replace_pointer()' over 'rcu_assign_pointer()' to update
'admin' immediately before an attempt to schedule freeing.

Fixes: a3d43c0d56f1 ("taprio: Add support adding an admin schedule")
Reported-by: syzbot+b65e0af58423fc8a73aa@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b65e0af58423fc8a73aa
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v5: unchanged since v4 but resend due to series change
v4: adjust subject to target net tree
v3: unchanged since v2
v2: unchanged since v1
---
 net/sched/sch_taprio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Vinicius Costa Gomes Sept. 5, 2024, 12:43 p.m. UTC | #1
Dmitry Antipov <dmantipov@yandex.ru> writes:

> In 'taprio_change()', 'admin' pointer may become dangling due to sched
> switch / removal caused by 'advance_sched()', and critical section
> protected by 'q->current_entry_lock' is too small to prevent from such
> a scenario (which causes use-after-free detected by KASAN). Fix this
> by prefer 'rcu_replace_pointer()' over 'rcu_assign_pointer()' to update
> 'admin' immediately before an attempt to schedule freeing.
>
> Fixes: a3d43c0d56f1 ("taprio: Add support adding an admin schedule")
> Reported-by: syzbot+b65e0af58423fc8a73aa@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=b65e0af58423fc8a73aa
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
> v5: unchanged since v4 but resend due to series change
> v4: adjust subject to target net tree
> v3: unchanged since v2
> v2: unchanged since v1
> ---

Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>


Cheers,
Dmitry Antipov Sept. 20, 2024, 9:05 a.m. UTC | #2
Just a friendly reminder. Note the net-next counterpart of this
series (https://lore.kernel.org/netdev/87wmjqnufp.fsf@intel.com/T/#t)
is already merged (both to net and net-next).

Dmitry

On 9/5/24 3:43 PM, Vinicius Costa Gomes wrote:
> Dmitry Antipov <dmantipov@yandex.ru> writes:
> 
>> In 'taprio_change()', 'admin' pointer may become dangling due to sched
>> switch / removal caused by 'advance_sched()', and critical section
>> protected by 'q->current_entry_lock' is too small to prevent from such
>> a scenario (which causes use-after-free detected by KASAN). Fix this
>> by prefer 'rcu_replace_pointer()' over 'rcu_assign_pointer()' to update
>> 'admin' immediately before an attempt to schedule freeing.
>>
>> Fixes: a3d43c0d56f1 ("taprio: Add support adding an admin schedule")
>> Reported-by: syzbot+b65e0af58423fc8a73aa@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=b65e0af58423fc8a73aa
>> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
>> ---
>> v5: unchanged since v4 but resend due to series change
>> v4: adjust subject to target net tree
>> v3: unchanged since v2
>> v2: unchanged since v1
>> ---
> 
> Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> 
> 
> Cheers,
diff mbox series

Patch

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index cc2df9f8c14a..59fad74d5ff9 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -1963,7 +1963,8 @@  static int taprio_change(struct Qdisc *sch, struct nlattr *opt,
 
 		taprio_start_sched(sch, start, new_admin);
 
-		rcu_assign_pointer(q->admin_sched, new_admin);
+		admin = rcu_replace_pointer(q->admin_sched, new_admin,
+					    lockdep_rtnl_is_held());
 		if (admin)
 			call_rcu(&admin->rcu, taprio_free_sched_cb);