[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 6 Apr 2016 11:03:17 -0700
From: Kees Cook <keescook@...omium.org>
To: Emrah Demir <ed@...sec.com>
Cc: LKML <linux-kernel@...r.kernel.org>, Dan Rosenberg <dan.j.rosenberg@...il.com>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>,
Linus Torvalds <torvalds@...ux-foundation.org>, Dave Jones <davej@...hat.com>
Subject: Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file
On Wed, Apr 6, 2016 at 6:03 AM, Emrah Demir <ed@...sec.com> wrote:
> From: Emrah Demir <ed@...sec.com>
Hi!
Thanks for sending this patch; I'm always glad to see new faces
helping. :) I have a few comments inline and a larger suggestion at
the end.
> Even though KASLR is aiming to mitigate remote attacks, with a simple LFI vulnerability through a web application, local leaks become as important as remote ones.
Be sure to 80-char wrap your commit logs (and code), as it makes
reading it easier. Running your patch through scripts/checkpatch.pl
would remind you:
WARNING: Possible unwrapped commit description (prefer a maximum 75
chars per line)
#5:
Even though KASLR is aiming to mitigate remote attacks, with a simple
LFI vulnerability through a web application, local leaks become as
important as remote ones.
> On the KASLR enabled systems in order to achieve expected protection, some files are needed to edited/modified to prevent leaks.
>
> /proc/iomem file leaks offset of text section. By adding 0x80000000, Attackers can get _text base address. KASLR will be bypassed.
Luckily, this will become less of a problem once the x86 virtual
memory randomization code lands, but I think there's enough in this
file that it should be protected.
>
> $ cat /proc/iomem | grep 'Kernel code'
> 38600000-38b7fe92 : Kernel code
> $ python -c 'print hex(0x38600000 + 0x80000000)'
> 0xb8600000
> # cat /proc/kallsyms | grep 'T _text'
> ffffffffb8600000 T _text
>
> By this patch after insertion resources, start and end address are zeroed. /proc/iomem and /proc/ioports sources, which use request_resource and insert_resource now shown as 0 value.
>
> Signed-off-by: Emrah Demir <ed@...sec.com>
> ---
> kernel/resource.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 2e78ead..5b9937e 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -321,6 +321,8 @@ int request_resource(struct resource *root, struct resource *new)
> struct resource *conflict;
>
> conflict = request_resource_conflict(root, new);
> + new->start = 0;
> + new->end = 0;
> return conflict ? -EBUSY : 0;
> }
>
> @@ -864,6 +866,8 @@ int insert_resource(struct resource *parent, struct resource *new)
> struct resource *conflict;
>
> conflict = insert_resource_conflict(parent, new);
> + new->start = 0;
> + new->end = 0;
> return conflict ? -EBUSY : 0;
> }
> EXPORT_SYMBOL_GPL(insert_resource);
This entirely eliminates the reporting, which I don't think is a good
idea as developers and debuggers would like to be able to see this
information. I would prefer that either /proc/iomem be unreadable to
non-root users or that the values be reported using %pK to let the
kptr_restrict control the output.
diff --git a/kernel/resource.c b/kernel/resource.c
index 2e78ead30934..8d5dd1dc9489 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -162,8 +162,8 @@ static const struct file_operations
proc_iomem_operations = {
static int __init ioresources_init(void)
{
- proc_create("ioports", 0, NULL, &proc_ioports_operations);
- proc_create("iomem", 0, NULL, &proc_iomem_operations);
+ proc_create("ioports", S_IRUSR, NULL, &proc_ioports_operations);
+ proc_create("iomem", S_IRUSR, NULL, &proc_iomem_operations);
return 0;
}
__initcall(ioresources_init);
And if this breaks things, make the S_IRUSR conditional on the
CONFIG_RANDOMIZE_BASE?
-Kees
--
Kees Cook
Chrome OS & Brillo Security
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
