question about /proc/<PID>/mem in 2.4

question about /proc/<PID>/mem in 2.4

[Tigran Aivazian]

Hello,

I noticed that in 2.4.x kernels the fs/proc/base.c:mem_read() function has 
this permission check:

        if (!MAY_PTRACE(task) || !may_ptrace_attach(task))
                return -ESRCH;

Are you sure it shouldn't be like this instead:

        if (!MAY_PTRACE(task) && !may_ptrace_attach(task))
                return -ESRCH;

Because, normally MAY_PTRACE() is 0 (i.e. for any process worth looking at :)
so may_ptrace_attach() is never even called.

Is there any reason for the above check on each read(2)? Shouldn't there 
be a simple check at ->open() time only? I assume this is to close some 
obscure "security hole" but I would like to see the explanation of how 
could any problem arise if a) such check wasn't done at all (except at 
open(2) time) or at least b) there was && instead of ||.

The 2.6.x situation is similar except the addition of the security stuff.

Kind regards
Tigran

Re: question about /proc/<PID>/mem in 2.4

[FabF]

On Mon, 2004-07-05 at 15:27, Tigran Aivazian wrote:
> Hello,
> 
> I noticed that in 2.4.x kernels the fs/proc/base.c:mem_read() function has 
> this permission check:
> 
>         if (!MAY_PTRACE(task) || !may_ptrace_attach(task))
>                 return -ESRCH;
> 
> Are you sure it shouldn't be like this instead:
> 
>         if (!MAY_PTRACE(task) && !may_ptrace_attach(task))
>                 return -ESRCH;
> 
> Because, normally MAY_PTRACE() is 0 (i.e. for any process worth looking at :)
> so may_ptrace_attach() is never even called.
> 
MAY_PTRACE is 1 normally AFAICS.The check as it stands wants both to
have non zero returns so is more restrictive than the one you're asking
for.

> Is there any reason for the above check on each read(2)? Shouldn't there 
> be a simple check at ->open() time only? I assume this is to close some 
> obscure "security hole" but I would like to see the explanation of how 
> could any problem arise if a) such check wasn't done at all (except at 
> open(2) time) or at least b) there was && instead of ||.
cf. chmod thread.

> 
> The 2.6.x situation is similar except the addition of the security stuff.
> 
> Kind regards
> Tigran
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: question about /proc/<PID>/mem in 2.4

[Tigran Aivazian]

On Mon, 5 Jul 2004, FabF wrote:
> > I noticed that in 2.4.x kernels the fs/proc/base.c:mem_read() function has 
> > this permission check:
> > 
> >         if (!MAY_PTRACE(task) || !may_ptrace_attach(task))
> >                 return -ESRCH;
> > 
> > Are you sure it shouldn't be like this instead:
> > 
> >         if (!MAY_PTRACE(task) && !may_ptrace_attach(task))
> >                 return -ESRCH;
> > 
> > Because, normally MAY_PTRACE() is 0 (i.e. for any process worth looking at :)
> > so may_ptrace_attach() is never even called.
> > 
> MAY_PTRACE is 1 normally AFAICS.The check as it stands wants both to
> have non zero returns so is more restrictive than the one you're asking
> for.

MAY_PTRACE is defined as:

#define MAY_PTRACE(task) \
        (task == current || \
        (task->parent == current && \
        (task->ptrace & PT_PTRACED) && task->state == TASK_STOPPED))

so, if a process (current) is interested in another process (task) which
is not itself and not one of its children then MAY_PTRACE is 0. and the
test in mem_read() immediately returns ESRCH error without checking
may_ptrace_attach() at all. I questioned this behaviour as being too
restrictive and would like to know the reason for it.

Surely, the super user (i.e. CAP_SYS_PTRACE in this context) should be 
allowed to read any process' memory without having to do the 
PTRACE_ATTACH/PTRACE_PEEKUSER kind of thing which strace(8) is doing?

Kind regards
Tigran

Re: question about /proc/<PID>/mem in 2.4

[FabF]

On Mon, 2004-07-05 at 16:22, Tigran Aivazian wrote:
> On Mon, 5 Jul 2004, FabF wrote:
> > > I noticed that in 2.4.x kernels the fs/proc/base.c:mem_read() function has 
> > > this permission check:
> > > 
> > >         if (!MAY_PTRACE(task) || !may_ptrace_attach(task))
> > >                 return -ESRCH;
> > > 
> > > Are you sure it shouldn't be like this instead:
> > > 
> > >         if (!MAY_PTRACE(task) && !may_ptrace_attach(task))
> > >                 return -ESRCH;
> > > 
> > > Because, normally MAY_PTRACE() is 0 (i.e. for any process worth looking at :)
> > > so may_ptrace_attach() is never even called.
> > > 
> > MAY_PTRACE is 1 normally AFAICS.The check as it stands wants both to
> > have non zero returns so is more restrictive than the one you're asking
> > for.
> 
> MAY_PTRACE is defined as:
> 
> #define MAY_PTRACE(task) \
>         (task == current || \
>         (task->parent == current && \
>         (task->ptrace & PT_PTRACED) && task->state == TASK_STOPPED))
> 
> so, if a process (current) is interested in another process (task) which
> is not itself and not one of its children then MAY_PTRACE is 0. and the
> test in mem_read() immediately returns ESRCH error without checking
> may_ptrace_attach() at all. I questioned this behaviour as being too
> restrictive and would like to know the reason for it.
> 
> Surely, the super user (i.e. CAP_SYS_PTRACE in this context) should be 
> allowed to read any process' memory without having to do the 
> PTRACE_ATTACH/PTRACE_PEEKUSER kind of thing which strace(8) is doing?

FYI may_ptrace_attach plugged somewhere between 2.4.21 & 22.This one get
used as is (ie without MAY_PTRACE) in proc_pid_environ but dunno about
reason why CAP_SYS_PTRACE isn't authoritative elsewhere.

Regards,
FabF



> 
> Kind regards
> Tigran
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: question about /proc/<PID>/mem in 2.4

[Tigran Aivazian]

On Mon, 5 Jul 2004, FabF wrote:
> > Surely, the super user (i.e. CAP_SYS_PTRACE in this context) should be 
> > allowed to read any process' memory without having to do the 
> > PTRACE_ATTACH/PTRACE_PEEKUSER kind of thing which strace(8) is doing?
> 
> FYI may_ptrace_attach plugged somewhere between 2.4.21 & 22.This one get
> used as is (ie without MAY_PTRACE) in proc_pid_environ but dunno about
> reason why CAP_SYS_PTRACE isn't authoritative elsewhere.

Ok, but still nobody seems to know why the super user is not allowed to
access /proc/<PID>/mem of any task. Any code which nobody in the world
knows the reason for, is broken and should be removed.

I will wait a few weeks to see if someone does come up with the reason for
that "extra secure" check in mem_read() and if nobody has objections I'll
send Linus a patch to relax the check to a more reasonable one, namely to
allow CAP_SYS_PTRACE process to bypass any other conditions imposed.

Kind regards
Tigran

Re: question about /proc/<PID>/mem in 2.4

[Arjan van de Ven]

[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]

On Tue, 2004-07-06 at 13:14, Tigran Aivazian wrote:
> On Mon, 5 Jul 2004, FabF wrote:
> > > Surely, the super user (i.e. CAP_SYS_PTRACE in this context) should be 
> > > allowed to read any process' memory without having to do the 
> > > PTRACE_ATTACH/PTRACE_PEEKUSER kind of thing which strace(8) is doing?
> > 
> > FYI may_ptrace_attach plugged somewhere between 2.4.21 & 22.This one get
> > used as is (ie without MAY_PTRACE) in proc_pid_environ but dunno about
> > reason why CAP_SYS_PTRACE isn't authoritative elsewhere.
> 
> Ok, but still nobody seems to know why the super user is not allowed to
> access /proc/<PID>/mem of any task. Any code which nobody in the world
> knows the reason for, is broken and should be removed.
> 
> I will wait a few weeks to see if someone does come up with the reason for
> that "extra secure" check in mem_read() and if nobody has objections I'll
> send Linus a patch to relax the check to a more reasonable one, namely to
> allow CAP_SYS_PTRACE process to bypass any other conditions imposed.

may I ask what the point is ?

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: question about /proc/<PID>/mem in 2.4

[Tigran Aivazian]

On Tue, 6 Jul 2004, Arjan van de Ven wrote:
> may I ask what the point is ?

Yes, sure. I asked about the point of this check in the function 
fs/proc/base.c:mem_read() (in 2.4 but 2.6 is similar):

           if (!MAY_PTRACE(task) || !may_ptrace_attach(task))
                return -ESRCH;

If you check the definition of MAY_PTRACE() macro and may_ptrace_attach() 
function then you will notice that if a typical root process (uid=euid=0)
tries to read /proc/<PID>/mem file for a process other than itself or one 
of its children then MAY_TRACE() will return 0 and therefore the above 
check will be if (1 || !may_ptrace_attach(task)) and thus evaluate to 1 
and return -ESRCH.

Therefore, even a privileged process (with CAP_SYS_PTRACE capability) is 
not allowed to read arbitrary process' /proc/<PID>/mem file.

This can be worked around by writing a (GPL of course) module but I didn't
want to spend time writing it (although actually I went ahead and started
writing it yesterday evening anyway :) if the above check is erroneous and
can simply be relaxed to allow root to read it. This will save me time and
effort, that's all.

But if the above check is there for a good reason, then I would like to 
know what that reason is, exactly.

Kind regards
Tigran

Re: question about /proc/<PID>/mem in 2.4

[Marcelo Tosatti]

On Tue, Jul 06, 2004 at 12:14:04PM +0100, Tigran Aivazian wrote:
> On Mon, 5 Jul 2004, FabF wrote:
> > > Surely, the super user (i.e. CAP_SYS_PTRACE in this context) should be 
> > > allowed to read any process' memory without having to do the 
> > > PTRACE_ATTACH/PTRACE_PEEKUSER kind of thing which strace(8) is doing?
> > 
> > FYI may_ptrace_attach plugged somewhere between 2.4.21 & 22.This one get
> > used as is (ie without MAY_PTRACE) in proc_pid_environ but dunno about
> > reason why CAP_SYS_PTRACE isn't authoritative elsewhere.
> 
> Ok, but still nobody seems to know why the super user is not allowed to
> access /proc/<PID>/mem of any task. Any code which nobody in the world
> knows the reason for, is broken and should be removed.
> 
> I will wait a few weeks to see if someone does come up with the reason for
> that "extra secure" check in mem_read() and if nobody has objections I'll
> send Linus a patch to relax the check to a more reasonable one, namely to
> allow CAP_SYS_PTRACE process to bypass any other conditions imposed.

Hi Tigran, 

This code was added to stop the ptrace/kmod vulnerabilities. I do not 
fully understand the issues around tsk->is_dumpable and the fix itself,
but I agree on that the checks here could be relaxed for the super user.

However changing it to 

        if (!is_dumpable(task) && !capable(CAP_SYS_PTRACE))
                goto out;

Seems wrong because this will stop always honoring the tsk->is_dumpable flag. (?)

Alan for sure can make the picture clear - he wrote this thing.
                                                                                                                                                                                   

Re: question about /proc/<PID>/mem in 2.4

[Tigran Aivazian]

On Tue, 6 Jul 2004, Marcelo Tosatti wrote:
> On Tue, Jul 06, 2004 at 12:14:04PM +0100, Tigran Aivazian wrote:
> > Ok, but still nobody seems to know why the super user is not allowed to
> > access /proc/<PID>/mem of any task. Any code which nobody in the world
> > knows the reason for, is broken and should be removed.
> > 
> > I will wait a few weeks to see if someone does come up with the reason for
> > that "extra secure" check in mem_read() and if nobody has objections I'll
> > send Linus a patch to relax the check to a more reasonable one, namely to
> > allow CAP_SYS_PTRACE process to bypass any other conditions imposed.
> 
> Hi Tigran, 
> 
> This code was added to stop the ptrace/kmod vulnerabilities. I do not 
> fully understand the issues around tsk->is_dumpable and the fix itself,
> but I agree on that the checks here could be relaxed for the super user.
> 
> However changing it to 
> 
>         if (!is_dumpable(task) && !capable(CAP_SYS_PTRACE))
>                 goto out;
> 
> Seems wrong because this will stop always honoring the tsk->is_dumpable flag. (?)
> 
> Alan for sure can make the picture clear - he wrote this thing.

Ok, let's see what Alan says then (if he has time to look at it).

Btw, the mem_read() code in both 2.4 and 2.6 can be cleaned up to use
get_task_mm() inline instead of doing it inline "manually".

Kind regards
Tigran

Re: question about /proc/<PID>/mem in 2.4

[Alan Cox]

On Tue, Jul 06, 2004 at 02:08:04PM +0100, Tigran Aivazian wrote:
> > This code was added to stop the ptrace/kmod vulnerabilities. I do not 
> > fully understand the issues around tsk->is_dumpable and the fix itself,
> > but I agree on that the checks here could be relaxed for the super user.

is_dumpable tells you various things in 2.4, including whether the
curent memory image is valid, and as a race preventor for ptrace during
exec of setuid apps. You should probably talk to Solar Designer about
the whole design of the dump/suid race fixing work rather than me.

We also had to deal with another nasty case which could be fixed by grabbing
the mm at open time (which then opens a resource attack bug).

Consider what happens if your setuid app reads stdin

	setuidapp < /proc/self/mem

(No idea how 2.6 deals with these but if its got better backportable ways
 that *actually work* it might make sense).

Re: question about /proc/<PID>/mem in 2.4

[Tigran Aivazian]

On Tue, 6 Jul 2004, Alan Cox wrote:

> On Tue, Jul 06, 2004 at 02:08:04PM +0100, Tigran Aivazian wrote:
> > > This code was added to stop the ptrace/kmod vulnerabilities. I do not 
> > > fully understand the issues around tsk->is_dumpable and the fix itself,
> > > but I agree on that the checks here could be relaxed for the super user.
> 
> is_dumpable tells you various things in 2.4, including whether the
> curent memory image is valid, and as a race preventor for ptrace during
> exec of setuid apps. You should probably talk to Solar Designer about
> the whole design of the dump/suid race fixing work rather than me.
> 
> We also had to deal with another nasty case which could be fixed by grabbing
> the mm at open time (which then opens a resource attack bug).
> 
> Consider what happens if your setuid app reads stdin
> 
> 	setuidapp < /proc/self/mem
> 
> (No idea how 2.6 deals with these but if its got better backportable ways
>  that *actually work* it might make sense).

Hello,

Ok, I followed the advice of Alan Cox and looked up "Solar Designer" on 
google.

So, my questions (to Solar Designer) are:

1. what exactly are the details of the setuid race Alan Cox is referring
to above? I have 0 (zero) doubts in the validity of Alan's words and
always trust him, but it doesn't imply that I always understand him, and
so would appreciate a clarification.

2. are you sure that there is no way to fix the above race without
preventing even the superuser access to /proc/<PID>/mem of 
any process?

More specifically (to save you time re-reading this thread) I am referring 
to the code in fs/proc/base.c:mem_read():

        if (!MAY_PTRACE(task) || !may_ptrace_attach(task))
                return -ESRCH;

and also the same check on return from access_process_vm(), which I 
suggested to relax a little, by allowing CAP_SYS_PTRACE user (or 
CAP_SYS_RAWIO perhaps?) to access /proc/<PID>/mem of any task without any 
hindrance.

Btw, the second check looks like an obvious race to me. I.e. if this
condition can change between the check in the beginning of mem_read() and
return from access_process_vm() then what is to stop it from changing just
after this second check and return from mem_read()?

Therefore, the code in mem_read() does look broken to me, but instead of 
"re-inventing the wheel" and trying to fix it "from scratch" I would like 
to get a fair knowledge of the history/reasons for these changes, please.

Kind regards
Tigran

Re: question about /proc/<PID>/mem in 2.4

[Tigran Aivazian]

the address solar@openwall.dot.com bounced back and on some mailing list
"Solar Designer" set his email address to be solar@false,com which I
assume is false...

Ok, maybe this guy, whoever he or she is, is reading linux-kernel messages
and will respond eventually :)

Many thanks to everyone who responded to my questions.

Kind regards
Tigran

On Wed, 7 Jul 2004, Tigran Aivazian wrote:

> On Tue, 6 Jul 2004, Alan Cox wrote:
> 
> > On Tue, Jul 06, 2004 at 02:08:04PM +0100, Tigran Aivazian wrote:
> > > > This code was added to stop the ptrace/kmod vulnerabilities. I do not 
> > > > fully understand the issues around tsk->is_dumpable and the fix itself,
> > > > but I agree on that the checks here could be relaxed for the super user.
> > 
> > is_dumpable tells you various things in 2.4, including whether the
> > curent memory image is valid, and as a race preventor for ptrace during
> > exec of setuid apps. You should probably talk to Solar Designer about
> > the whole design of the dump/suid race fixing work rather than me.
> > 
> > We also had to deal with another nasty case which could be fixed by grabbing
> > the mm at open time (which then opens a resource attack bug).
> > 
> > Consider what happens if your setuid app reads stdin
> > 
> > 	setuidapp < /proc/self/mem
> > 
> > (No idea how 2.6 deals with these but if its got better backportable ways
> >  that *actually work* it might make sense).
> 
> Hello,
> 
> Ok, I followed the advice of Alan Cox and looked up "Solar Designer" on 
> google.
> 
> So, my questions (to Solar Designer) are:
> 
> 1. what exactly are the details of the setuid race Alan Cox is referring
> to above? I have 0 (zero) doubts in the validity of Alan's words and
> always trust him, but it doesn't imply that I always understand him, and
> so would appreciate a clarification.
> 
> 2. are you sure that there is no way to fix the above race without
> preventing even the superuser access to /proc/<PID>/mem of 
> any process?
> 
> More specifically (to save you time re-reading this thread) I am referring 
> to the code in fs/proc/base.c:mem_read():
> 
>         if (!MAY_PTRACE(task) || !may_ptrace_attach(task))
>                 return -ESRCH;
> 
> and also the same check on return from access_process_vm(), which I 
> suggested to relax a little, by allowing CAP_SYS_PTRACE user (or 
> CAP_SYS_RAWIO perhaps?) to access /proc/<PID>/mem of any task without any 
> hindrance.
> 
> Btw, the second check looks like an obvious race to me. I.e. if this
> condition can change between the check in the beginning of mem_read() and
> return from access_process_vm() then what is to stop it from changing just
> after this second check and return from mem_read()?
> 
> Therefore, the code in mem_read() does look broken to me, but instead of 
> "re-inventing the wheel" and trying to fix it "from scratch" I would like 
> to get a fair knowledge of the history/reasons for these changes, please.
> 
> Kind regards
> Tigran
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: question about /proc/<PID>/mem in 2.4

[Daniel Jacobowitz]

On Wed, Jul 07, 2004 at 02:26:41PM +0100, Tigran Aivazian wrote:
> the address solar@openwall.dot.com bounced back and on some mailing list

The dot is not supposed to be included literally....

> "Solar Designer" set his email address to be solar@false,com which I
> assume is false...

In fact it isn't false....


-- 
Daniel Jacobowitz

Re: question about /proc/<PID>/mem in 2.4

[Alan Cox]

On Wed, Jul 07, 2004 at 02:53:44PM +0100, Tigran Aivazian wrote:
>         if (!MAY_PTRACE(task) || !may_ptrace_attach(task))
>                 return -ESRCH;
> 
> Btw, the second check looks like an obvious race to me. I.e. if this
> condition can change between the check in the beginning of mem_read() and
> return from access_process_vm() then what is to stop it from changing just
> after this second check and return from mem_read()?

The boundary in question is exec(). We don't allow ptrace attach during
an exec in process and that may well make the specific mm untraceable.
So the lifetime is something like

	exec
		no attach
		create mm
		load stuff
		destroy old mm
		if suid
			no ptrace
		can attach (maybe)

So an open by self before an exec always refers to the old dead mm and
the one after always hits the other checks. A user can ptrace their own
app but ptrace itself doesn't set ptraced exec's setuid.