From patchwork Thu Jul 16 15:19:42 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Thompson X-Patchwork-Id: 235649 Delivered-To: patches@linaro.org Received: by 2002:a92:d244:0:0:0:0:0 with SMTP id v4csp888612ilg; Thu, 16 Jul 2020 08:20:07 -0700 (PDT) X-Received: by 2002:a05:6000:d0:: with SMTP id q16mr5933596wrx.166.1594912807501; Thu, 16 Jul 2020 08:20:07 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1594912807; cv=none; d=google.com; s=arc-20160816; b=bxt4nBD23R8EmLTE5snMZoTMSOF7sdymuT53hXPxW3pRVcc3m+1dgAKvOM/i2ZJodp 4MHrzvSbKeln/qOMtU49cCQK2yZoUncgosb/EQkYQCx8J5ivEtIEIw0NV4LTdqjtw3kr 5cnvxl6boeGILEBAR9OjB0pmisQJ9M6IbjMwIRyNat0Ta9CWz3u9zn6MGardGy9CuxN8 phNc3ZOLOOQ83kSadQvVz9AiuH0Z9m4Gz5JAemmbqaAgx06Fm1ZUFyNi/uyxNyZpt2hd jQA5vAdP4rwG0LkIRhUcGRWoLnOMGztG7xItOxlmZukJzCdwsM16EhWOXcQ6eaYQKHV4 /01g== 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=MA+kP4DlJmfdXBdpErlC9E+irTIJBNObkffFIf2y82I=; b=VlTAHd6YmuJwydR0KjLVb93PN4+aJv9oj3gSygbLhT5Tf5rC6kVLQ8xqiMppVhmX7p oG8MAQlzQu1hOLBmILYjLtcpZjWK2Gdl7ZGBSuOlWRJqY0EF1wReAhQA15mRS+5muOPh zwLbu7uI2YAdbHp9mop+ubZRAkh+PqSNO6xtSPFUvsy5InQijLHoUNIK0jZV0vh69Hor g680VGW9Jfzh3IHqCLNRoS2UuwULCxav93TpeTXTFpHWNk7k7XCPihb54YKmmSKGxptV HDOl9yvOEuMxiCzMdEF9D8dt2kH/7N35cMld+9Ct/xnIYKSidMeM0d4kdZ4goDWcgTGc U7nw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=JqIq+nSS; 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 f26sor3057850wmc.1.2020.07.16.08.20.07 for (Google Transport Security); Thu, 16 Jul 2020 08:20:07 -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=JqIq+nSS; 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=MA+kP4DlJmfdXBdpErlC9E+irTIJBNObkffFIf2y82I=; b=JqIq+nSSgbqT32taOoagK4GvW1IjeH/ECen5VaQpTPssRfgi2K/8DOcl6JBC0QxDeW L9hLCUvRYISHnV89d2sJRdD+6c5sxQkufYhP3jxVx68jsxG0TdCDZiP70lmDH+wZdRTC FznHjEsKE7OT6A1QXZ5HM+aeuVDeWOUeieYNavx54TIHRGrU3K6MgOS/grSopWEWTAaO hlCL5R4TMZFizDEe8gDbVEiz3XHHsc3Q22RNnsXKbadgu4t28iDUBY7TwTIPfdOnEURW 2jL1tZurcc5gb75TDGPuPCEicPFYNSgzjJ08eexA4pnUfomSRpk/TrpZMqsicqzagoh1 vmYQ== 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=MA+kP4DlJmfdXBdpErlC9E+irTIJBNObkffFIf2y82I=; b=kaEYhKlItWamKYG5aQkLmOZoI8UEfu8I7JjkFXJ9/MJG1Kbzyy+Ve8p+8IJiy+lYaD sr+tVaKDuCfL1oAIK2KEJE5DvbJS6uogTGNj6me99nwySqG2QqWaoDRgtoZtoxJy2vgq 1hQNxjDlYXh3M6TcPse6rZtGk1XpIEERq0CED0h84jnDVR4YlpVKiFxpjysVCQlQDpx2 o5bct9AYp96FE1ObRX4jWr5iQLhkNvDSUQE4i0xxdJ5sg+mwcU1V6Qjvs6dmprUp/NjF BJeFi43tOtOQ/yCbRBvhE2ccJPyYpps4u2eAXYTfT1/4NQvBDNSSCzRCtFjHjIY8zYux m+KA== X-Gm-Message-State: AOAM532rxlESheQZPVwGwn5ERm92R8mpgIVqhXUwE5Koh90WMD3Poear n1bxw0RrUf3IYzF8/pbPkRikQ9rx X-Google-Smtp-Source: ABdhPJwWZzro9ckYK2greM7KjTK8oMd0961HXp/aw47Rj9PySVK21oCV8YKb2OYPEOZ0KHUJkUyCTw== X-Received: by 2002:a7b:c38c:: with SMTP id s12mr4799097wmj.136.1594912807056; Thu, 16 Jul 2020 08:20:07 -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 u17sm9877687wrp.70.2020.07.16.08.20.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 16 Jul 2020 08:20:06 -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, Masami Hiramatsu , kgdb-bugreport@lists.sourceforge.net, linux-kernel@vger.kernel.org, patches@linaro.org Subject: [PATCH v2 2/3] kgdb: Use the kprobe blocklist to limit single stepping Date: Thu, 16 Jul 2020 16:19:42 +0100 Message-Id: <20200716151943.2167652-3-daniel.thompson@linaro.org> X-Mailer: git-send-email 2.25.4 In-Reply-To: <20200716151943.2167652-1-daniel.thompson@linaro.org> References: <20200716151943.2167652-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 blocklist. 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 7caba4604edc..aefe823998cb 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h @@ -214,6 +214,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 133a361578dc..4b59bcc90c5d 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -208,6 +208,19 @@ 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 breakpointing... stepping won't be safe + * either! + */ + if (kgdb_within_blocklist(addr)) + return -EINVAL; + + return 0; +} + unsigned long __weak kgdb_arch_pc(int exception, struct pt_regs *regs) { return instruction_pointer(regs); diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c index 61774aec46b4..f1c88007cc2b 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 5c7949061671..cd40bf780b93 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); }