linux-kernel - [PATCH v2] stackleak: Fix a race between stack erasing sysctl handlers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20200828031928.43584-1-songmuchun@bytedance.com>
Date:   Fri, 28 Aug 2020 11:19:28 +0800
From:   Muchun Song <songmuchun@...edance.com>
To:     keescook@...omium.org, mhiramat@...nel.org, rostedt@...dmis.org,
        alex.popov@...ux.com, miguel.ojeda.sandonis@...il.com
Cc:     linux-kernel@...r.kernel.org,
        Muchun Song <songmuchun@...edance.com>
Subject: [PATCH v2] stackleak: Fix a race between stack erasing sysctl handlers

There is a race between the assignment of `table->data` and write value
to the pointer of `table->data` in the __do_proc_doulongvec_minmax() on
the other thread.

    CPU0:                                 CPU1:
                                          proc_sys_write
    stack_erasing_sysctl                    proc_sys_call_handler
      table->data = &state;                   stack_erasing_sysctl
                                                table->data = &state;
      proc_doulongvec_minmax
        do_proc_doulongvec_minmax             sysctl_head_finish
          __do_proc_doulongvec_minmax           unuse_table
            i = table->data;
            *i = val;  // corrupt CPU1's stack

Fix this by duplicating the `table`, and only update the duplicate of
it.

Fixes: 964c9dff0091 ("stackleak: Allow runtime disabling of kernel stack erasing")
Signed-off-by: Muchun Song <songmuchun@...edance.com>
---
changelogs in v2:
 1. Add more details about how the race happened to the commit message.

 kernel/stackleak.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/kernel/stackleak.c b/kernel/stackleak.c
index a8fc9ae1d03d..fd95b87478ff 100644
--- a/kernel/stackleak.c
+++ b/kernel/stackleak.c
@@ -25,10 +25,15 @@ int stack_erasing_sysctl(struct ctl_table *table, int write,
 	int ret = 0;
 	int state = !static_branch_unlikely(&stack_erasing_bypass);
 	int prev_state = state;
+	struct ctl_table dup_table = *table;
 
-	table->data = &state;
-	table->maxlen = sizeof(int);
-	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
+	/*
+	 * In order to avoid races with __do_proc_doulongvec_minmax(), we
+	 * can duplicate the @table and alter the duplicate of it.
+	 */
+	dup_table.data = &state;
+	dup_table.maxlen = sizeof(int);
+	ret = proc_dointvec_minmax(&dup_table, write, buffer, lenp, ppos);
 	state = !!state;
 	if (ret || !write || state == prev_state)
 		return ret;
-- 
2.11.0