Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1372543 > unrolled thread

[PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file

Back to article view | Back to linux.kernel

Contents

  [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file Emrah Demir <ed@abdsec.com> - 2016-04-06 16:00 +0200
    Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-06 17:30 +0200
      Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-06 20:00 +0200
        Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file ed@abdsec.com - 2016-04-06 20:10 +0200
          Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file Kees Cook <keescook@chromium.org> - 2016-04-06 20:30 +0200
          Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file Kees Cook <keescook@chromium.org> - 2016-04-06 20:40 +0200
            Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-06 20:50 +0200
              Re: [kernel-hardening] Re: [PATCH] KERNEL: resource: Fix bug on  leakage in /proc/iomem file Yves-Alexis Perez <corsac@debian.org> - 2016-04-06 21:00 +0200
                Re: [kernel-hardening] Re: [PATCH] KERNEL: resource: Fix bug on  leakage in /proc/iomem file Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-06 21:10 +0200
                  Re: [kernel-hardening] Re: [PATCH] KERNEL: resource: Fix bug on  leakage in /proc/iomem file Borislav Petkov <bp@alien8.de> - 2016-04-06 21:20 +0200
                  Re: [kernel-hardening] Re: [PATCH] KERNEL: resource: Fix bug on  leakage in /proc/iomem file Yves-Alexis Perez <corsac@debian.org> - 2016-04-06 21:20 +0200
                  Re: [kernel-hardening] Re: [PATCH] KERNEL: resource: Fix bug on  leakage in /proc/iomem file Ingo Molnar <mingo@kernel.org> - 2016-04-06 23:50 +0200
              Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file Bjørn Mork <bjorn@mork.no> - 2016-04-06 21:30 +0200
          Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-06 20:40 +0200
          Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file Kees Cook <keescook@chromium.org> - 2016-04-06 21:00 +0200
          Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem  file Christian Kujau <lists@nerdbynature.de> - 2016-04-06 21:10 +0200
        Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-06 23:20 +0200
          Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file Kees Cook <keescook@chromium.org> - 2016-04-06 23:30 +0200
            Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-06 23:40 +0200
          Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file Kees Cook <keescook@chromium.org> - 2016-04-14 06:30 +0200
            Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file Emrah Demir <ed@abdsec.com> - 2016-04-14 11:40 +0200
    Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file Kees Cook <keescook@chromium.org> - 2016-04-06 20:10 +0200

Page 1 of 2  [1] 2  Next page →

#1372543 — [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file

From: Emrah Demir <ed@abdsec.com>

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.

$ 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@abdsec.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);
-- 
2.8.0.rc3

[toc] | [next] | [standalone]

#1372611

On Wed, Apr 6, 2016 at 6:03 AM, Emrah Demir <ed@abdsec.com> wrote:
>
> By this patch after insertion resources, start and end address are zeroed.

I'd much rather just not insert the resources in the first place then.

         Linus

[toc] | [prev] | [next] | [standalone]

#1372706

[Multipart message — attachments visible in raw view] — view raw

On Wed, Apr 6, 2016 at 8:20 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I'd much rather just not insert the resources in the first place then.

So I'd find a patch like the attached to be perfectly acceptable (in
fact, we should have done this long ago).

That said, for a kernel hardening thing, I think it would be much more
important to just make sure that KASLR is enabled much more. Right now
I think it's disabled in practice if you enable hibernation support,
and I think most distros do that.

So I think that in *practice*, kaslr is much more likely to be
defeated by much more mundane reasons.

             Linus

[toc] | [prev] | [next] | [standalone]

#1372710

First, I wrote your attached patch, but then I thought zeroing other 
/proc/iomem values would be better. So I changed it.

Most distros don't use KASLR, but they use kptr_restrict. Without KASLR, 
kptr_restirct most likely useless. As you said these things should be 
done long ago


         Emrah Demir

[toc] | [prev] | [next] | [standalone]

#1372724

On Wed, Apr 6, 2016 at 11:05 AM,  <ed@abdsec.com> wrote:
> First, I wrote your attached patch, but then I thought zeroing other
> /proc/iomem values would be better. So I changed it.
>
> Most distros don't use KASLR, but they use kptr_restrict. Without KASLR,

Well, hopefully that'll change over time. :)

> kptr_restirct most likely useless. As you said these things should be done
> long ago

This results in a warning, but the kernel's printf formatting supports it:

kernel/resource.c: In function ‘r_show’:
kernel/resource.c:118:4: warning: '0' flag used with ‘%p’ gnu_printf
format [-Wformat=]

I'm not sure how to best suppress that...

diff --git a/kernel/resource.c b/kernel/resource.c
index 2e78ead30934..d5881d143fb6 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -111,10 +111,10 @@ static int r_show(struct seq_file *m, void *v)
        for (depth = 0, p = r; depth < MAX_IORES_LEVEL; depth++, p = p->parent)
                if (p->parent == root)
                        break;
-       seq_printf(m, "%*s%0*llx-%0*llx : %s\n",
+       seq_printf(m, "%*s%0*pK-%0*pK : %s\n",
                        depth * 2, "",
-                       width, (unsigned long long) r->start,
-                       width, (unsigned long long) r->end,
+                       width, (void *) r->start,
+                       width, (void *) r->end,
                        r->name ? r->name : "<BAD>");
        return 0;
 }

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1372725

On Wed, Apr 6, 2016 at 11:31 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Wed, Apr 6, 2016 at 11:05 AM,  <ed@abdsec.com> wrote:
>>
>> Most distros don't use KASLR, but they use kptr_restrict. Without KASLR,
>> kptr_restirct most likely useless.
>
> Well, yes kaslr is effectively useless right now due to the fact that
> people still use hibernation in effectively every single distro out
> there.

At some point I'd like to see if distros would be interested in
inverting the default logic (maybe with a CONFIG to avoid changing the
current behavior) where instead of needing to put "kaslr" on the
command line to prefer kaslr over hibernation, end users would need to
add "nokaslr" to perfer hibernation for that boot. (Bike shed config
name: CONFIG_RANDOMIZE_BASE_ENABLED.)

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1372731

On Wed, Apr 6, 2016 at 11:37 AM, Kees Cook <keescook@chromium.org> wrote:
>
> At some point I'd like to see if distros would be interested in
> inverting the default logic (maybe with a CONFIG to avoid changing the
> current behavior) where instead of needing to put "kaslr" on the
> command line to prefer kaslr over hibernation, end users would need to
> add "nokaslr" to perfer hibernation for that boot. (Bike shed config
> name: CONFIG_RANDOMIZE_BASE_ENABLED.)

I suspect there really aren't all that many hibernation users out
there at all, and that yes, that would be the right default.

Hibernation is really quite nasty when you have to have a fairly big
special partition for it, and shrink your memory down. Writing things
to disk was a whole lot more reasonable back in the days when laptops
had 16MB of memory.

I really wonder how many people use it with a modern laptop and
distro. I doubt it's much faster than just rebooting the whole system
anyway, and there are lots of downsides.

Maybe we could even just do the default switch in the kernel
independently of any distro people, and see if anybody even _notices_.

               Linus

[toc] | [prev] | [next] | [standalone]

#1372733 — Re: [kernel-hardening] Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file

[Multipart message — attachments visible in raw view] — view raw

On mer., 2016-04-06 at 11:43 -0700, Linus Torvalds wrote:
> Hibernation is really quite nasty when you have to have a fairly big
> special partition for it, and shrink your memory down. Writing things
> to disk was a whole lot more reasonable back in the days when laptops
> had 16MB of memory.

Actually you just have to have a swap partition, which people still set as
more or less the ram size, I think, so all in all it works (especially if
people hibernate without the ram completely used).
> 
> I really wonder how many people use it with a modern laptop and
> distro. I doubt it's much faster than just rebooting the whole system
> anyway, and there are lots of downsides.

I quite never hibernate on my laptop (it wouldn't work anyway since I boot wit
kaslr and have PAX_SANITIZE), but I use hibernation on desktops where suspend
to ram doesn't work because of radeon or nvidia graphic card (actually suspend
usually works, resume doesn't). If/when suspend to ram works fine, I think
hibernation is mostly useless.

Regards,
-- 
Yves-Alexis

[toc] | [prev] | [next] | [standalone]

#1372742 — Re: [kernel-hardening] Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file

On Wed, Apr 6, 2016 at 11:53 AM, Yves-Alexis Perez <corsac@debian.org> wrote:
>
> Actually you just have to have a swap partition, which people still set as
> more or less the ram size, I think, so all in all it works (especially if
> people hibernate without the ram completely used).

I guess people still do those. I have one on my laptop, but that's
because I only have 4GB of RAM in that thing. I'd never hibernate it,
though. If I can't just get it back from suspend immediately, I'd
rather just boot it from scratch.

On bigger machines where I have 16GB or more, I tend to go "I'd rather
fail early and perhaps buy more RAM than see the slowdown or write to
my precious ssd". I guess I might have a swap partition just because a
distro did one for me and I didn't catch it.

So yeah, maybe swap partitions are still more common than I thought.
And I didn't even consider the possibility that people would hibernate
a desktop like you do.

             Linus

[toc] | [prev] | [next] | [standalone]

#1372750 — Re: [kernel-hardening] Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file

On Wed, Apr 06, 2016 at 09:11:07PM +0200, Yves-Alexis Perez wrote:
> On mer., 2016-04-06 at 12:02 -0700, Linus Torvalds wrote:
> > So yeah, maybe swap partitions are still more common than I thought.
> > And I didn't even consider the possibility that people would hibernate
> > a desktop like you do.
> 
> To be fair, it's *my* use case, because suspend won't work but I'm lazy and
> don't like to reopen everything I was doing, but still have to shut the
> machine down once in a while (or to save power).

I'm doing exactly the same thing with my workstation:

echo 3 > /proc/sys/vm/drop_caches
echo "shutdown" > /sys/power/disk
echo "disk" > /sys/power/state

*exactly* because I don't want to restart everything. :-)

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1372752 — Re: [kernel-hardening] Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file

[Multipart message — attachments visible in raw view] — view raw

On mer., 2016-04-06 at 12:02 -0700, Linus Torvalds wrote:
> So yeah, maybe swap partitions are still more common than I thought.
> And I didn't even consider the possibility that people would hibernate
> a desktop like you do.

To be fair, it's *my* use case, because suspend won't work but I'm lazy and
don't like to reopen everything I was doing, but still have to shut the
machine down once in a while (or to save power). And *I* would gladly trade
hibernation for kaslr and PAX_SANITIZE on those desktops.

Regards,
-- 
Yves-Alexis

[toc] | [prev] | [next] | [standalone]

#1372875 — Re: [kernel-hardening] Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> So yeah, maybe swap partitions are still more common than I thought. And I 
> didn't even consider the possibility that people would hibernate a desktop like 
> you do.

Also many distros will hibernate automatically on critically low battery (when 
suspend won't save the system).

It would be much better to fix the kASLR/hibernation incompatibility ...

Just a random guess: much of the hibernation incompatibility comes from the fact 
that on hibernation bootups the kASLR seed changes, which breaks hibernated kernel 
addresses, right?

That should be easy to fix: if we added a kaslr_seed=xyz boot option, and added 
that parmeter automatically (without showing it in /proc/cmdline ;-) on 
hibernation bootups, we could solve much of the incompatibility, right?

This means that the first 'cold' bootup would set the kASLR seed - and subsequent 
hibernated bootups would inherit it. That should be perfectly OK as long as we 
don't expose the seed somewhere.

We could also write the kASLR seed to the hibernation image, but I don't think we 
have the value available early enough - a boot option is better.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1372759

Linus Torvalds <torvalds@linux-foundation.org> writes:

> I suspect there really aren't all that many hibernation users out
> there at all, and that yes, that would be the right default.
>
> Hibernation is really quite nasty when you have to have a fairly big
> special partition for it, and shrink your memory down. Writing things
> to disk was a whole lot more reasonable back in the days when laptops
> had 16MB of memory.
>
> I really wonder how many people use it with a modern laptop and
> distro. I doubt it's much faster than just rebooting the whole system
> anyway, and there are lots of downsides.

Huh? Do modern laptops now have infinite battery capacity?  I regularily
use hibernation as emergency shutdown when the battery runs out.  You
mean that never happens to other people?  Or you just take the hit and
stops everything you were currently doing, saving every open file etc?


Bjørn

[toc] | [prev] | [next] | [standalone]

#1372727

On Wed, Apr 6, 2016 at 11:05 AM,  <ed@abdsec.com> wrote:
>
> Most distros don't use KASLR, but they use kptr_restrict. Without KASLR,
> kptr_restirct most likely useless.

Well, yes kaslr is effectively useless right now due to the fact that
people still use hibernation in effectively every single distro out
there.

But kptr_restrict was enabled by distro people, and in theory it does
end up possibly helping: it at least it hides the exact per-function
addresses.

Of course, with 99.9% of all users then using a distro kernel, you can
just get those remotely anyway by just downloading the distro image,
so it turns out that now there is effectively zero bits that you are
really hiding, because the information is effectively right there in
"uname -a".

End result: kptr_restrict is a wonderful flag if all you want to
disable is a trivial convenience function that is easy for an attacker
to do other ways.

Quite frankly, personally I find a lot of security people and patches
to be disingenuous for exactly this kind of reason. They look at the
small details, and are completely missing the big picture.

I'm at the IoT conference right now. "Security" has been a big word
this week. "45 billion devices, lack of security, the sky is falling".
I don't think we had a lot of people talking about "oh, the cloud
service is getting shut down, so now those devices don't even *work*".

But that's ok. Because "security" is more important than "reality". Groan.

                Linus

[toc] | [prev] | [next] | [standalone]

#1372740

On Wed, Apr 6, 2016 at 11:52 AM, Christian Kujau <lists@nerdbynature.de> wrote:
> On Wed, 6 Apr 2016, ed@abdsec.com wrote:
>> First, I wrote your attached patch, but then I thought zeroing other
>> /proc/iomem values would be better. So I changed it.
>
> On my systems, /proc/iomem, /proc/ioports and others get their
> world-readable bits removed during bootup - I guess that would mitigate
> this issue too?

Yeah, I think that'd be sufficient (that's the first patch I
suggested). It's not a strong as kptr_restrict since kptr_restrict has
mode "2", but ... I think that's some diminishing returns...

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1372744 — Re: [PATCH] KERNEL: resource: Fix bug on leakage in /proc/iomem file

On Wed, 6 Apr 2016, ed@abdsec.com wrote:
> First, I wrote your attached patch, but then I thought zeroing other
> /proc/iomem values would be better. So I changed it.

On my systems, /proc/iomem, /proc/ioports and others get their 
world-readable bits removed during bootup - I guess that would mitigate 
this issue too?

Christian.
-- 
BOFH excuse #184:

loop found in loop in redundant loopback

[toc] | [prev] | [next] | [standalone]

#1372834

On Wed, Apr 6, 2016 at 10:54 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> So I'd find a patch like the attached to be perfectly acceptable (in
> fact, we should have done this long ago).

I just committed it, let's see if some odd program uses the iomem
data. I doubt it, and I always enjoy improvements that remove more
lines of code than they add.

              Linus

[toc] | [prev] | [next] | [standalone]

#1372843

On Wed, Apr 6, 2016 at 2:19 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Wed, Apr 6, 2016 at 10:54 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> So I'd find a patch like the attached to be perfectly acceptable (in
>> fact, we should have done this long ago).
>
> I just committed it, let's see if some odd program uses the iomem
> data. I doubt it, and I always enjoy improvements that remove more
> lines of code than they add.

Hrm, okay. I still think just changing the perms would be less
troublesome. Knowing where the kernel is in physical memory when
debugging physical memory issues seems important to me. But, I have
never actually used it since I prefer looking in kallsyms. :)

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1372874

On Wed, Apr 6, 2016 at 2:27 PM, Kees Cook <keescook@chromium.org> wrote:
>
> Hrm, okay. I still think just changing the perms would be less
> troublesome.

No, that would be much *more* trouble-some, because we have things
like bug-reporting documentation that tells people to send /proc/iomem
etc information on crashes. There may well be scripts like that out
there.

So it's much more likely that just removing the "Kernel code" etc
lines is not going to break anything.

            Linus

[toc] | [prev] | [next] | [standalone]

#1378473

On Wed, Apr 6, 2016 at 2:19 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Wed, Apr 6, 2016 at 10:54 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> So I'd find a patch like the attached to be perfectly acceptable (in
>> fact, we should have done this long ago).
>
> I just committed it, let's see if some odd program uses the iomem
> data. I doubt it, and I always enjoy improvements that remove more
> lines of code than they add.

Hrm, it looks like at least Ubuntu's kernel security test suite
expects to find these entries (when it verifies that STRICT_DEVMEM
hasn't regressed). Also, the commit only removed the entries on x86.
Most (all?) of the other architectures still have them. Could you
revert this for now, and I'll cook up a %pK-based solution for -next?

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web