From patchwork Fri Jun 5 13:21:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Thompson X-Patchwork-Id: 187508 Delivered-To: patches@linaro.org Received: by 2002:a92:cf06:0:0:0:0:0 with SMTP id c6csp2377973ilo; Fri, 5 Jun 2020 06:22:25 -0700 (PDT) X-Received: by 2002:a5d:6305:: with SMTP id i5mr9390485wru.268.1591363345055; Fri, 05 Jun 2020 06:22:25 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1591363345; cv=none; d=google.com; s=arc-20160816; b=m5yKxhlj3lIJwfbPvG7ornbpbgO5nEs/3B9h9BJpkzn4sLs6IF9HnV0dNpfVkvvfaI zq5BCnwLBhOlChODPPKd05PzkINKwWh07FE9+KaddEvrT7SqJgBiP2UCx/G9VKBJESQN 6EXpD+YRiO1A/JnmqXuGFZ/ZVElDnSqf4PKtHPlHv3QlMmSlt6GTtl5I0M90zO/fulNN FbW0B+pKmP9Xr4puOfQ2DkE0H0rj7p+is1CsxFSuoCoRIjpmjYyJuebUGjoKc416LKXy N0+UOvYOfM8gezXaFS7Etiv+TL1adiz4wKmwknHsKkYPQHxZSHYVrU4nGSfQRcwZ1Wkm SZVA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:dkim-signature; bh=0Wd49qJJfphbkQd0zc5Sg32oNGChg1bIC5PBSadN6hM=; b=nweZmeLSD1m7CI11JOQqoZPp7+fwnSHQOjhQkbmRdZpSnvxL9AcElIAfJRVMZ/q+XL v/3u0Sv2DxW5puciOjF7aXdBzKq9nE8/UV2OBSOaMdk6aaExLZw2jZI0WQTSW0h5JraH SFnXXROlZKv5qHb0Ak0fW498yg903iVSNQzPPbqDMynCvoij0OQ19UJJchBa6hPMiXPP WI4iOscOYno0d552e5s4M+L0LCoHJRAu8QT34mQ6JolkVSH17FOD7zc7H5kJpHk3rCIl 35ZRdxdYuoEFE85A8FhalHLI4Fun6ED1vncwxjssAPJgySx+6NCWYA/kX/ELXZIRGS93 NmXQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=Ki1mWzQU; spf=pass (google.com: domain of daniel.thompson@linaro.org designates 209.85.220.65 as permitted sender) smtp.mailfrom=daniel.thompson@linaro.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from mail-sor-f65.google.com (mail-sor-f65.google.com. [209.85.220.65]) by mx.google.com with SMTPS id q13sor5354367wra.46.2020.06.05.06.22.24 for (Google Transport Security); Fri, 05 Jun 2020 06:22:25 -0700 (PDT) Received-SPF: pass (google.com: domain of daniel.thompson@linaro.org designates 209.85.220.65 as permitted sender) client-ip=209.85.220.65; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=Ki1mWzQU; spf=pass (google.com: domain of daniel.thompson@linaro.org designates 209.85.220.65 as permitted sender) smtp.mailfrom=daniel.thompson@linaro.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=0Wd49qJJfphbkQd0zc5Sg32oNGChg1bIC5PBSadN6hM=; b=Ki1mWzQUC6JcLvWE/8DkDtIIdjhYtPuZOCRYcToxv8LWR6pRb4lGurJyBIOkPuorTR Bsi8zComYX+iO1sRCteKaKGKyFUrV6SX6hjPxqnASoH0k1DXUJXEKyc3HGx2ueMIE2qb uywx8D1qZVnXSdiKKLdwXvNKLMNFIU9Bu8JHD5HLksHo/nlTZTGo8jsu+Ah8CXeONqWi hvJhuA1ziakt2ACYLpIUSNY1wMXCd97jniit9A4a3KTg0apr2Lf0h7yCyZefMoacoO2C 9q2d2vkvbL2DdSsZcTpTomTgUQeXMVAHG5xrferfVgiIys/NkQQv1KlcJViHEKl8u1a0 kXFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=0Wd49qJJfphbkQd0zc5Sg32oNGChg1bIC5PBSadN6hM=; b=CeSvYEhra7UdAcMe4J3O+I0f+jWxT593RCzIBAHZ9igWYIiEwSF8dvQ8vwkavlXWG6 exRorTeOE9g5VzfqYuD1uyR0lboUGuHigpSMhkpiUi0ytQEHv7tFXPYJbgxdh4L8a+SQ yALC3hm7Gy50ukeMbmn/7DPAZBGQvdKsxQcDp15ApcqhfD2Ep5kVmo/nS1wRZjd6dnc1 ypyVlYZJquvlfs3ALjuu88BgfoJocqjGwEX4qzckcV9HTNBgk09gOFqPfsumEgD6LJn0 f9kyUmzG8xRBfBQ/jaCE62N3fHBo51HjDybwDiezwCC+3UdGQHx8SBxUqZR/RhJX/7lh RRlg== X-Gm-Message-State: AOAM533elaFzaJ77KzcVclHp1Dm9HeIEpDe8V200UYZgfR52TX5G/Bea qCae3KQkvlHdEitnmSVXnlgHh0R4 X-Google-Smtp-Source: ABdhPJzpXOyKm+//aW91zcmMraLFE6hXoKeh/4G6ONWrnUgxSkq+Twz2EbRA9faf1wyV0IhllSZGdQ== X-Received: by 2002:a5d:4701:: with SMTP id y1mr9434014wrq.310.1591363344584; Fri, 05 Jun 2020 06:22:24 -0700 (PDT) Return-Path: Received: from wychelm.lan (cpc141214-aztw34-2-0-cust773.18-1.cable.virginm.net. [86.9.19.6]) by smtp.gmail.com with ESMTPSA id 1sm11419211wmz.13.2020.06.05.06.22.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 05 Jun 2020 06:22:24 -0700 (PDT) From: Daniel Thompson To: Jason Wessel , Douglas Anderson Cc: Daniel Thompson , Peter Zijlstra , sumit.garg@linaro.org, pmladek@suse.com, sergey.senozhatsky@gmail.com, will@kernel.org, kgdb-bugreport@lists.sourceforge.net, linux-kernel@vger.kernel.org, patches@linaro.org Subject: [RFC PATCH 2/4] kgdb: Use the kprobe blacklist to limit single stepping Date: Fri, 5 Jun 2020 14:21:28 +0100 Message-Id: <20200605132130.1411255-3-daniel.thompson@linaro.org> X-Mailer: git-send-email 2.25.4 In-Reply-To: <20200605132130.1411255-1-daniel.thompson@linaro.org> References: <20200605132130.1411255-1-daniel.thompson@linaro.org> MIME-Version: 1.0 If we are running in a part of the kernel that dislikes breakpoint debugging then it is very unlikely to be safe to single step. Add some safety rails to prevent stepping through anything on the kprobe blacklist. As part of this kdb_ss() will no longer set the DOING_SS flags when it requests a step. This is safe because this flag is already redundant, returning KDB_CMD_SS is all that is needed to request a step (and this saves us from having to unset the flag if the safety check fails). Signed-off-by: Daniel Thompson --- include/linux/kgdb.h | 1 + kernel/debug/debug_core.c | 13 +++++++++++++ kernel/debug/gdbstub.c | 10 +++++++++- kernel/debug/kdb/kdb_bp.c | 8 ++------ kernel/debug/kdb/kdb_main.c | 10 ++++++++-- 5 files changed, 33 insertions(+), 9 deletions(-) -- 2.25.4 diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h index c62d76478adc..93b612d81714 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h @@ -213,6 +213,7 @@ extern void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc); /* Optional functions. */ extern int kgdb_validate_break_address(unsigned long addr); +extern int kgdb_validate_single_step_address(unsigned long addr); extern int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt); extern int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt); diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index 81f56d616e04..4a2df4509fe1 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -215,6 +215,18 @@ int __weak kgdb_validate_break_address(unsigned long addr) return err; } +int __weak kgdb_validate_single_step_address(unsigned long addr) +{ + /* + * Disallow stepping when we are executing code that is marked + * as unsuitable for kprobing. + */ + if (within_kprobe_blacklist(addr)) + return -EINVAL; + + return 0; +} + unsigned long __weak kgdb_arch_pc(int exception, struct pt_regs *regs) { return instruction_pointer(regs); @@ -1192,6 +1204,7 @@ noinline void kgdb_breakpoint(void) atomic_dec(&kgdb_setting_breakpoint); } EXPORT_SYMBOL_GPL(kgdb_breakpoint); +NOKPROBE_SYMBOL(kgdb_breakpoint); static int __init opt_kgdb_wait(char *str) { diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c index 4b280fc7dd67..beb73a61a16d 100644 --- a/kernel/debug/gdbstub.c +++ b/kernel/debug/gdbstub.c @@ -1041,8 +1041,16 @@ int gdb_serial_stub(struct kgdb_state *ks) if (tmp == 0) break; /* Fall through - on tmp < 0 */ - case 'c': /* Continue packet */ case 's': /* Single step packet */ + error = kgdb_validate_single_step_address( + kgdb_arch_pc(ks->ex_vector, + ks->linux_regs)); + if (error != 0) { + error_packet(remcom_out_buffer, error); + break; + } + fallthrough; + case 'c': /* Continue packet */ if (kgdb_contthread && kgdb_contthread != current) { /* Can't switch threads in kgdb */ error_packet(remcom_out_buffer, -EINVAL); diff --git a/kernel/debug/kdb/kdb_bp.c b/kernel/debug/kdb/kdb_bp.c index ec4940146612..4853c413f579 100644 --- a/kernel/debug/kdb/kdb_bp.c +++ b/kernel/debug/kdb/kdb_bp.c @@ -507,18 +507,14 @@ static int kdb_bc(int argc, const char **argv) * None. * Remarks: * - * Set the arch specific option to trigger a debug trap after the next - * instruction. + * KDB_CMD_SS is a command that our caller acts on to effect the step. */ static int kdb_ss(int argc, const char **argv) { if (argc != 0) return KDB_ARGCOUNT; - /* - * Set trace flag and go. - */ - KDB_STATE_SET(DOING_SS); + return KDB_CMD_SS; } diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c index ec190569f690..4b277c066f48 100644 --- a/kernel/debug/kdb/kdb_main.c +++ b/kernel/debug/kdb/kdb_main.c @@ -1189,7 +1189,7 @@ static int kdb_local(kdb_reason_t reason, int error, struct pt_regs *regs, kdb_dbtrap_t db_result) { char *cmdbuf; - int diag; + int diag, res; struct task_struct *kdb_current = kdb_curr_task(raw_smp_processor_id()); @@ -1346,10 +1346,16 @@ static int kdb_local(kdb_reason_t reason, int error, struct pt_regs *regs, } if (diag == KDB_CMD_GO || diag == KDB_CMD_CPU - || diag == KDB_CMD_SS || diag == KDB_CMD_KGDB) break; + if (diag == KDB_CMD_SS) { + res = kgdb_validate_single_step_address(instruction_pointer(regs)); + if (res == 0) + break; + diag = res; + } + if (diag) kdb_cmderror(diag); }