Hi,
/proc/timer_list contains kernel addresses, like e.g.:
#0: <c000000001404158>, tick_sched_timer, S:01, .tick_nohz_restart_sched_tick, swapper/0
...
Avoid leaking them to user space to make writing kernel exploits a bit harder.
(I currently cannot think of a userland tool that uses this, this is
likely pretty much root-only.)
Ciao, Marcus
Signed-off-by: Marcus Meissner <[email protected]>
---
kernel/time/timer_list.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/time/timer_list.c b/kernel/time/timer_list.c
index ab8f5e3..5ae1ce3 100644
--- a/kernel/time/timer_list.c
+++ b/kernel/time/timer_list.c
@@ -293,7 +293,7 @@ static int __init init_timer_list_procfs(void)
{
struct proc_dir_entry *pe;
- pe = proc_create("timer_list", 0444, NULL, &timer_list_fops);
+ pe = proc_create("timer_list", 0400, NULL, &timer_list_fops);
if (!pe)
return -ENOMEM;
return 0;
--
1.7.1
On Wed, 2010-11-17 at 18:08 +0100, Marcus Meissner wrote:
> Hi,
>
> /proc/timer_list contains kernel addresses, like e.g.:
> #0: <c000000001404158>, tick_sched_timer, S:01, .tick_nohz_restart_sched_tick, swapper/0
> ...
>
> Avoid leaking them to user space to make writing kernel exploits a bit harder.
>
> (I currently cannot think of a userland tool that uses this, this is
> likely pretty much root-only.)
iirc powertop parses this..
On Wed, Nov 17, 2010 at 06:18:32PM +0100, Peter Zijlstra wrote:
> On Wed, 2010-11-17 at 18:08 +0100, Marcus Meissner wrote:
> > Hi,
> >
> > /proc/timer_list contains kernel addresses, like e.g.:
> > #0: <c000000001404158>, tick_sched_timer, S:01, .tick_nohz_restart_sched_tick, swapper/0
> > ...
> >
> > Avoid leaking them to user space to make writing kernel exploits a bit harder.
> >
> > (I currently cannot think of a userland tool that uses this, this is
> > likely pretty much root-only.)
>
> iirc powertop parses this..
powertop already says on startup:
PowerTOP needs to be run as root to collect enough information
And as developer tool it usually is for people having root access.
Ciao, Marcus
On Wed, 2010-11-17 at 18:21 +0100, Marcus Meissner wrote:
> On Wed, Nov 17, 2010 at 06:18:32PM +0100, Peter Zijlstra wrote:
> > On Wed, 2010-11-17 at 18:08 +0100, Marcus Meissner wrote:
> > > Hi,
> > >
> > > /proc/timer_list contains kernel addresses, like e.g.:
> > > #0: <c000000001404158>, tick_sched_timer, S:01, .tick_nohz_restart_sched_tick, swapper/0
> > > ...
> > >
> > > Avoid leaking them to user space to make writing kernel exploits a bit harder.
> > >
> > > (I currently cannot think of a userland tool that uses this, this is
> > > likely pretty much root-only.)
> >
> > iirc powertop parses this..
>
> powertop already says on startup:
>
> PowerTOP needs to be run as root to collect enough information
I know, but you said you didn't know of a tool that used the file.. now
you do ;-)
On Wed, 17 Nov 2010 18:18:32 +0100
Peter Zijlstra <[email protected]> wrote:
> On Wed, 2010-11-17 at 18:08 +0100, Marcus Meissner wrote:
> > Hi,
> >
> > /proc/timer_list contains kernel addresses, like e.g.:
> > #0: <c000000001404158>, tick_sched_timer, S:01, .tick_nohz_restart_sched_tick, swapper/0
> > ...
> >
> > Avoid leaking them to user space to make writing kernel exploits a bit harder.
> >
> > (I currently cannot think of a userland tool that uses this, this is
> > likely pretty much root-only.)
>
> iirc powertop parses this..
I bet it doesn't look at the kernel address (why was that added in the
first place, anyway?)
I'd suggest that the risk of breakage would be much less if we left the
file permissions alone and arranged for those addresses to be
0000000000000000 for non-root readers.
On Wed, 17 Nov 2010, Andrew Morton wrote:
> On Wed, 17 Nov 2010 18:18:32 +0100
> Peter Zijlstra <[email protected]> wrote:
>
> > On Wed, 2010-11-17 at 18:08 +0100, Marcus Meissner wrote:
> > > Hi,
> > >
> > > /proc/timer_list contains kernel addresses, like e.g.:
> > > #0: <c000000001404158>, tick_sched_timer, S:01, .tick_nohz_restart_sched_tick, swapper/0
> > > ...
> > >
> > > Avoid leaking them to user space to make writing kernel exploits a bit harder.
> > >
> > > (I currently cannot think of a userland tool that uses this, this is
> > > likely pretty much root-only.)
> >
> > iirc powertop parses this..
>
> I bet it doesn't look at the kernel address (why was that added in the
> first place, anyway?)
>
> I'd suggest that the risk of breakage would be much less if we left the
> file permissions alone and arranged for those addresses to be
> 0000000000000000 for non-root readers.
You beat me to it. Having the full information is quite helpful at
times.
Thanks,
tglx
* Thomas Gleixner <[email protected]> wrote:
> On Wed, 17 Nov 2010, Andrew Morton wrote:
>
> > On Wed, 17 Nov 2010 18:18:32 +0100
> > Peter Zijlstra <[email protected]> wrote:
> >
> > > On Wed, 2010-11-17 at 18:08 +0100, Marcus Meissner wrote:
> > > > Hi,
> > > >
> > > > /proc/timer_list contains kernel addresses, like e.g.:
> > > > #0: <c000000001404158>, tick_sched_timer, S:01, .tick_nohz_restart_sched_tick, swapper/0
> > > > ...
> > > >
> > > > Avoid leaking them to user space to make writing kernel exploits a bit harder.
> > > >
> > > > (I currently cannot think of a userland tool that uses this, this is
> > > > likely pretty much root-only.)
> > >
> > > iirc powertop parses this..
> >
> > I bet it doesn't look at the kernel address (why was that added in the
> > first place, anyway?)
> >
> > I'd suggest that the risk of breakage would be much less if we left the
> > file permissions alone and arranged for those addresses to be
> > 0000000000000000 for non-root readers.
>
> You beat me to it. Having the full information is quite helpful at
> times.
We should do something like:
if (!capable(CAP_SYS_ADMIN))
print_ptr = NULL;
sprintf(s, "%p", print_ptr);
Thanks,
Ingo
Le jeudi 18 novembre 2010 à 09:12 +0100, Ingo Molnar a écrit :
> We should do something like:
>
> if (!capable(CAP_SYS_ADMIN))
> print_ptr = NULL;
>
> sprintf(s, "%p", print_ptr);
A while ago, Thomas suggested following idea :
<quote>
Considering the amount of duplication you are about to do, you may
want to think about adding a new pointer format extension then. We
already have special '%p' modes for IPv6 addresse, MAC addresses and
various other pointer types. It wouldn't be hard to add one which
does not print (null).
</quote>
http://www.spinics.net/lists/netdev/msg146606.html
So the if (!capable(CAP_SYS_ADMIN)) could be centralized in %pX handling
(X being a new letter, not actually 'X')
Hi!
> > > /proc/timer_list contains kernel addresses, like e.g.:
> > > #0: <c000000001404158>, tick_sched_timer, S:01, .tick_nohz_restart_sched_tick, swapper/0
> > > ...
> > >
> > > Avoid leaking them to user space to make writing kernel exploits a bit harder.
> > >
> > > (I currently cannot think of a userland tool that uses this, this is
> > > likely pretty much root-only.)
> >
> > iirc powertop parses this..
>
> powertop already says on startup:
>
> PowerTOP needs to be run as root to collect enough information
>
> And as developer tool it usually is for people having root access.
.....and now you are actively decreasing security.
Yes, developers usally have root access, but no, developers do not
like to work as root.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi!
> > > > iirc powertop parses this..
> > >
> > > I bet it doesn't look at the kernel address (why was that added in the
> > > first place, anyway?)
> > >
> > > I'd suggest that the risk of breakage would be much less if we left the
> > > file permissions alone and arranged for those addresses to be
> > > 0000000000000000 for non-root readers.
> >
> > You beat me to it. Having the full information is quite helpful at
> > times.
>
> We should do something like:
>
> if (!capable(CAP_SYS_ADMIN))
> print_ptr = NULL;
>
> sprintf(s, "%p", print_ptr);
Having single file that contains different stuff based on capable() is
very very ugly.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html