From patchwork Wed Nov 8 00:59:56 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Torvalds X-Patchwork-Id: 10047497 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 793A460247 for ; Wed, 8 Nov 2017 01:00:15 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6B7A02A2C5 for ; Wed, 8 Nov 2017 01:00:15 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 601412A2D5; Wed, 8 Nov 2017 01:00:15 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by mail.wl.linuxfoundation.org (Postfix) with SMTP id 473C12A2C5 for ; Wed, 8 Nov 2017 01:00:13 +0000 (UTC) Received: (qmail 25612 invoked by uid 550); 8 Nov 2017 01:00:10 -0000 Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Delivered-To: mailing list kernel-hardening@lists.openwall.com Received: (qmail 24549 invoked from network); 8 Nov 2017 01:00:09 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=4DMunzqvCJB1x1K8RQFrHTvLXDvdo7CRROP3Lw1ti6k=; b=q7CleRtxMW+J86ReVPtGreBz2/B7/D41GIdeBaBX0IiUvLdKCqMej7N+8yOkLWMM+O ySZF1DK3teAhLnzhhO+ldFsrbHfk3oeIYlyDUspZGEbD8Nj9C5rtCvEge/Ev/Rb6fDpY /ZZfI9LzR1G4uoCC3WzcpTQZmH+NrCIWC/Uv4iZyKiPnaH+/JzC65IYCPQN1y6iQ1e22 rD99pTeZlTZLqY5DTjgzMRhDDPl6msR3NR9fSb/h/g26xUtPvk4fS63AjdWFw4rPYN9n YJVBiwaEUQq9l8gz0IPY5hF+5i9pjIW0faFjDit84dji0e8E9WBHyxWcPaU/lPIo60t5 fVoQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=4DMunzqvCJB1x1K8RQFrHTvLXDvdo7CRROP3Lw1ti6k=; b=DcettBgUWcVzMdY+q2svo1/Vb6GSc7OpIQXAXl6lmr+s7VDvqnyiYPIeAy0qsqE2/y k+Xs/7UXLB5bhMwdpeMXzSXdGhcfvluSABZwVFrJcjwm47/JTyLYPeemp5ihMmQTKsOy Key1m3oJWJm4YWcNUKFs/Oaovgit3WCZSJtTGo7jfMUisNZW6wROvXNb39vbEgI01Ck9 hOFFsbyZsD7ACXVbco6ZictA2qbaNT2nuuPv3+GIbLJpZ5FVJb2Exwp3hU43oh0g6fBj BhFs2Wzqq/siZD5GkfmTQ7hRhNIHGLsd9O1sBt8x0OSMcf7F3wMQyf0sYW42hv6sAscS pyRQ== X-Gm-Message-State: AJaThX4sMGzedbc3VGg1QCmwL2H20oLYCh1uzRviGJ43FwOB4Xz45D+N UZCJkwll4z1IoibuGKEyJCS4Y2ABT0hwHGluHis= X-Google-Smtp-Source: ABhQp+ROGno+/X060HnCrSCUdetu3URICr7PqrQLA3x+RUgcf2ZJMlVW7Rhw7xmXuMCKVwgXl1c3D0sPkMZ++8OGvoY= X-Received: by 10.107.164.76 with SMTP id n73mr773372ioe.175.1510102797233; Tue, 07 Nov 2017 16:59:57 -0800 (PST) MIME-Version: 1.0 Sender: linus971@gmail.com In-Reply-To: References: <1510050731-32446-1-git-send-email-me@tobin.cc> From: Linus Torvalds Date: Tue, 7 Nov 2017 16:59:56 -0800 X-Google-Sender-Auth: nRKg6IIvc4dRw1miG6lNmN-4UNQ Message-ID: To: Laura Abbott Cc: "Tobin C. Harding" , "kernel-hardening@lists.openwall.com" , "Jason A. Donenfeld" , "Theodore Ts'o" , Kees Cook , Paolo Bonzini , Tycho Andersen , "Roberts, William C" , Tejun Heo , Jordan Glover , Greg KH , Petr Mladek , Joe Perches , Ian Campbell , Sergey Senozhatsky , Catalin Marinas , Will Deacon , Steven Rostedt , Chris Fries , Dave Weinstein , Daniel Micay , Djalal Harouni , Linux Kernel Mailing List , Network Development , David Miller Subject: Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Nov 7, 2017 at 3:36 PM, Laura Abbott wrote: > > I'd probably put /proc/kallsyms and /proc/modules on the omit list > since those are designed to leak addresses to userspace. Well, they are indeed designed to leak addresses, but not a lot of people should care. So I think we could tighten them up. For example, maybe /proc/kallsyms could just default to not showing values to non-root users. We *did* originally try to use "kptr_restrict" with a default value of 1, it's just that it was never fixable on a case-by-case basis as people started saying "that breaks my flow, because xyz". But if we do it for one file at a time, we probably *can* try to fix complaints. Something like the attached TOTALLY UNTESTED patch. It's meant more as an RFC, not for application, but it's also meant to show how we can tailor the behavior for specific workflow issues. So take that "kallsyms_for_perf()" thing as an example of how we can say "hey, if you already have access to kernel profiling anyway, there's no point in hiding kallsyms". And there may be other similar things we can do. The situation with /proc/modules should be similar. Using kptr_restrict was a big hammer and might have broken something unrelated, but did anybody actually care about the particular case of /proc/modules not showing the module address to normal users? probably not. "lsmod" certainly doesn't care, and that's what people really want. Both /proc/kallsyms and /proc/modules _used_ to be really important for oops reporting, but that was long ago when the kernel didn't report symbol information of its own. So we have historical reasons for people to be able to read those files, but those are mainly things that aren't relevant (or even possible) on modern kernels anyway. So I don'r think we should omit /proc/kallsyms and /proc/modules - we should just fix them. The attached patch may not be good enough as is, but maybe something _like_ it will work well enough that people won't care? (And do note the "TOTALLY UNTESTED". It seems to compile. But maybe I got some test exactly the wrong way around and it doesn't actually _work_. Caveat testor). Linus kernel/kallsyms.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c index 127e7cfafa55..5b1299c1e4b0 100644 --- a/kernel/kallsyms.c +++ b/kernel/kallsyms.c @@ -480,6 +480,7 @@ struct kallsym_iter { char name[KSYM_NAME_LEN]; char module_name[MODULE_NAME_LEN]; int exported; + int show_value; }; static int get_ksymbol_mod(struct kallsym_iter *iter) @@ -580,14 +581,23 @@ static void s_stop(struct seq_file *m, void *p) { } +#ifndef CONFIG_64BIT +# define KALLSYM_FMT "%08lx" +#else +# define KALLSYM_FMT "%016lx" +#endif + static int s_show(struct seq_file *m, void *p) { + unsigned long value; struct kallsym_iter *iter = m->private; /* Some debugging symbols have no name. Ignore them. */ if (!iter->name[0]) return 0; + value = iter->show_value ? iter->value : 0; + if (iter->module_name[0]) { char type; @@ -597,10 +607,10 @@ static int s_show(struct seq_file *m, void *p) */ type = iter->exported ? toupper(iter->type) : tolower(iter->type); - seq_printf(m, "%pK %c %s\t[%s]\n", (void *)iter->value, + seq_printf(m, KALLSYM_FMT " %c %s\t[%s]\n", value, type, iter->name, iter->module_name); } else - seq_printf(m, "%pK %c %s\n", (void *)iter->value, + seq_printf(m, KALLSYM_FMT " %c %s\n", value, iter->type, iter->name); return 0; } @@ -612,6 +622,40 @@ static const struct seq_operations kallsyms_op = { .show = s_show }; +static inline int kallsyms_for_perf(void) +{ +#ifdef CONFIG_PERF_EVENTS + extern int sysctl_perf_event_paranoid; + if (sysctl_perf_event_paranoid <= 0) + return 1; +#endif + return 0; +} + +/* + * We show kallsyms information even to normal users if we've enabled + * kernel profiling and are explicitly not paranoid (so kptr_restrict + * is clear, and sysctl_perf_event_paranoid isn't set). + * + * Otherwise, require CAP_SYSLOG (assuming kptr_restrict isn't set to + * block even that). + */ +static int kallsyms_show_value(void) +{ + switch (kptr_restrict) { + case 0: + if (kallsyms_for_perf()) + return 1; + /* fallthrough */ + case 1: + if (has_capability_noaudit(current, CAP_SYSLOG)) + return 1; + /* fallthrough */ + default: + return 0; + } +} + static int kallsyms_open(struct inode *inode, struct file *file) { /* @@ -625,6 +669,7 @@ static int kallsyms_open(struct inode *inode, struct file *file) return -ENOMEM; reset_iter(iter, 0); + iter->show_value = kallsyms_show_value(); return 0; }