[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <E1GGQ8f-00053J-02@cab-20.cs.huji.ac.il>
Date: Fri, 25 Aug 2006 04:05:36 +0300 (IDT)
From: Amnon Shiloh <amnons@...huji.ac.il>
To: Frederik Deweerdt <deweerdt@...e.fr>
CC: "Eric W. Biederman" <ebiederm@...ssion.com>,
Amnon Shiloh <amnons@...huji.ac.il>,
linux-kernel@...r.kernel.org, akpm@...l.org, gregkh@...e.de
Subject: Re: [2.6.18 patch] fix mem_write return value (was: Re: bug report:
mem_write)
> On Thu, Aug 24, 2006 at 10:33:20AM -0600, Eric W. Biederman wrote:
> > Frederik Deweerdt <deweerdt@...e.fr> writes:
> >
> > > On Thu, Aug 24, 2006 at 11:25:37AM +0300, Amnon Shiloh wrote:
> > >> Hi,
> > >>
> > >> Alright, I know that "mem_write" (fs/proc/base.c) is a "security hazard",
> > >> but I need to use it anyway (as super-user only), and find it broken,
> > >> somewhere between Linux-2.6.17 and Linux-2.6.18-rc4.
> > >>
> > >> The point is that in the beginning of the routine, "copied" is set to 0,
> > >> but it is no good because in lines 805 and 812 it is set to other values.
> > >> Finally, the routine returns as if it copied 12 (=ENOMEM) bytes less than
> > >> it actually did.
> > > True, it looks like the faulty commit is:
> > > de7587343bfebc186995ad294e3de0da382eb9bc
> >
> > Actually it was: 99f895518368252ba862cc15ce4eb98ebbe1bec6
> > Which is what you url points to, odd.
> >
> > > http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff_plain;h=99f895518368252ba862cc15ce4eb98ebbe1bec6;hp=8578cea7509cbdec25b31d08b48a92fcc3b1a9e3
> > >
> > > The attached patch should fix it. Maybe that should go to 2.6.18.
> > > Thanks for the bug report,
> >
> > The patch looks correct. Although this won't cause anyone problems as the code
> > is disabled.
> Right, I missed this, so this is really not urgent.
> >
> > Signed-off-by: Eric Biederman <ebiederm@...ssion.com>
> >
> > As for enabling this. I believe we need an extra permission check just before
> > we copy the data from our temporary buffer to the target task, to ensure
> > nothing has changed. The history does not really capture why this code
> > was disabled, but before this gets enabled I would like to understand more
> > than just the comment. I believe with a little care this can be safely enabled
> > as it doesn't let you do anything ptrace wouldn't do, and it should let you do
> > it anytime except when ptrace would allow it. Thus not introducing any new
> > security holes.
> I've found two interesting links on that:
> http://lkml.org/lkml/2006/3/10/224
> and
> http://www.google.com/search?q=cache:4y8MWSuHOpIJ:files.security-protocols.com/kernelhacking/procpidmem.pdf&hl=en&ct=clnk&cd=3&client=firefox-a
> The second one in particular goes in great detail on why the author
> thinks this is dangerous, and what could be done to re-enable it.
>
> Regards,
> Frederik
>
I am aware of those risks, but since I desparately need this feature
and the program that needs it is SETUID-root anyway, I have it enabled
but added a test to make sure that only root can use it.
It works well and I can see no reason on earth how this could be a
security hazard when only called by the super-user.
Regards,
Amnon.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists