From: Bernhard Kaindl <bernhard.kai...@gmx.de>
Date: Wed, 23 Apr 2003 01:40:07 +0200
Local: Tues, Apr 22 2003 7:40 pm
Subject: [PATCH][2.4+ptrace] fix side effects of the kmod/ptrace secfix
I've attached a patch which fixes the remaining side effects
On Tue, 22 Apr 2003, Yusuf Wilajati Purna wrote:Sorry if my descriptions in my previos mail did not have any word too
> Thanks for the clarification. :-)
much(really short) but I tried to make the point straight for people
which know the code. I'm adding a little bit more verbosity now :-)
The check added by Alan's patch to ptrace_check_attach was:
+ if (!is_dumpable(child))
New, replacement check in ptrace_check_attach:
+ if (!child->task_dumpable)
I want to explain now, why the above use of is_dumpable() broke ptrace
is_dumpable() checks if both, task_dumpable and mm->dumpable are set, and
The new kernel_thread() function added by Alan's patch sets task_dumpable
By adding is_dumpable(child) to ptrace_check_attach(), the patch posted
So, removing the wrong check to child->mm->dumpable and only checking
The other place which needed to be touched to fix Alan's patch was
> >@@ -123,6 +127,8 @@ int access_process_vm(struct task_structaccess_process_vm() is in the same code patch as ptrace_check_attach.
> > /* Worry about races with exit() */
> > task_lock(tsk);
> > mm = tsk->mm;
> >+ if (!is_dumpable(tsk) || (&init_mm == mm))
> >+ mm = NULL;
> > if (mm)
> > atomic_inc(&mm->mm_users);
> > task_unlock(tsk);
If you read the sys_ptrace implementation in arch/i386/kernel/ptrace.c,
So the !is_dumpable(tsk) check above it just a repetition if the previous
> Just to recapitulate,Note, in addtion to breaking root's ability to trace setuid programs,
> The following changes to the original patch (Alan's patch):
> int ptrace_check_attach(struct task_struct *child, int kill)
> int access_process_vm(struct task_struct *tsk, unsigned long addr,
having the tsk->mm->dumpable checked by !is_dumpable(tsk) at this place
also broke /proc/PID/cmdline and /proc/PID/environ because access_process_vm()
is also used by these proc functions.
If somebody says this opens a securtiy leak, I'd have to say:
If a suid task leaks such information thru it's cmdline buffer, it's
You would need to restrict cmdline access to all root processes(not only
> ...Yes exacly, they do fix these side-effects, as your test correctly gives:
> can solve the following side-effects introduced by the original patch:
> - /proc/PID/cmdline and /proc/PID/environ are empty for non-dumpable
> - strace started by root cannot ptrace user threads or such non-dumpable
> At least, I have confirmed this on an i386/IA-32 platform. And I haveExactly, the change (effectve removal of the task->mm->dumpable flag check,
> checked also
> that ptrace/kmod exploits such as isec-ptrace-kmod-exploit.c, ptrace.c,
> km3.c cannot
> get root privilege with the changes.
which is not part of the kernel_thread/ptrace issue, fix the the two side
effects you describe above while maintaining same security against the
kmod/ptrace exploits because it only removes code that has nothing to
do with the kernel_thread/ptrace issue, which was added by Alan's patch
and introduced these side effects.
It's really that easy, you just have to look and the code and see it ;-)
Ok, you need to understand how the ptrace code works and how Alan's
It's just cleanup, nothing more, nothing very creative, but a nice
> Any comments?I'm sorry that I did not send a patch the first time to make the
change 100% clear to anybody, but I'm doing this now.
Incremental patch which applies on top of the patch posted by
With only this patch applied I'd think 2.4.21 could be released,
2.4.21-rc1, if it would be released as is, has the potential of
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.