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

Groups > linux.kernel > #1396858 > unrolled thread

Is it really correct to check for breakpoint in kernel space against ptracer's address space?

Back to article view | Back to linux.kernel

Contents

  Is it really correct to check for breakpoint in kernel space against  ptracer's address space? Ruslan Kabatsayev <b7.10110111@gmail.com> - 2016-05-09 10:50 +0200
    Re: Is it really correct to check for breakpoint in kernel space  against ptracer's address space? Andy Lutomirski <luto@kernel.org> - 2016-05-10 05:40 +0200
      Re: Is it really correct to check for breakpoint in kernel space  against ptracer's address space? Ruslan Kabatsayev <b7.10110111@gmail.com> - 2016-05-10 14:00 +0200
        Re: Is it really correct to check for breakpoint in kernel space  against ptracer's address space? Andy Lutomirski <luto@amacapital.net> - 2016-05-10 17:30 +0200
          Re: Is it really correct to check for breakpoint in kernel space  against ptracer's address space? Ruslan Kabatsayev <b7.10110111@gmail.com> - 2016-05-10 17:50 +0200
            Re: Is it really correct to check for breakpoint in kernel space  against ptracer's address space? Andy Lutomirski <luto@amacapital.net> - 2016-05-10 18:20 +0200

#1396858 — Is it really correct to check for breakpoint in kernel space against ptracer's address space?

Hello all.

Currently a 32-bit ptracer can't set HW breakpoints in tracee over
address space limitations of _tracer_. Even if the tracee is 64-bit,
doing PTRACE_POKEUSER into u_debugreg[n] with value>=0xffffe000 leads
to EINVAL (below is a test tracer program to reproduce this). At the
same time, if tracer is 64-bit, then for both 32- and 64-bit tracees
the PTRACE_POKEUSER call will succeed even if violates address space
constraints for tracee.

I've traced this to arch_check_bp_in_kernel_space() in
arch/x86/kernel/hw_breakpoint.c, which checks the address against
TASK_SIZE, which as I understood refers to the current task, i.e.
caller of the syscall, instead of the tracee (at least tracing this in
Bochs leads me to this conclusion).
This doesn't seem to make sense, since the breakpoint is going to be
active in the tracee, not in the tracer, so it seems the BP address
should be checked against tracee address space instead.
Am I wrong here?

Here's the test program. Its argument is taken to be path to tracee to
launch. If none, /bin/true is taken. If you compile it with -m32 and
try to launch any binary, be it 32- or 64-bit, you'll get the
following output:

Trying to write 0xffffe000 into DR0...ptrace(POKEUSER): Invalid argument
Trying to write 0xffffdfff into DR1...OK

If you instead compile it with -m64, the output for both 32- and
64-bit tracees will be:

Trying to write 0xffffe000 into DR0...OK
Trying to write 0xffffdfff into DR1...OK

// ---- start of test program -----
#define _POSIX_SOURCE
#include <stdio.h>
#include <errno.h>
#include <stdlib.h>
#include <stddef.h>
#include <sys/ptrace.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <sys/user.h>
#include <unistd.h>
#include <signal.h>

void test(pid_t pid,int n,long addr)
{
    fprintf(stderr,"Trying to write %#lx into DR%d...",addr,n);
    if(ptrace(PTRACE_POKEUSER,pid,offsetof(struct user,u_debugreg[n]),addr)==-1)
        perror("ptrace(POKEUSER)");
    else
        fputs("OK\n",stderr);
}

int main(int argc,char** argv)
{
    const char* progPath="/bin/true";
    if(argc>1) progPath=argv[1];

    const pid_t pid = fork();
    switch(pid)
    {
    case -1: perror("fork"); return 1;
    case 0:
        if(ptrace(PTRACE_TRACEME)==-1)
        {
            perror("ptrace(TRACEME)");
            abort();
        }
        execl(progPath,progPath,(char*)NULL);
        perror("execl");
        abort();
    default:
      {
        int status;
        if(waitpid(pid,&status,__WALL)==-1)
        {
            perror("waitpid");
            goto error;
        }

        if(WIFSTOPPED(status) && WSTOPSIG(status) == SIGABRT)
            return 1;
        if(!WIFSTOPPED(status)||WSTOPSIG(status)!=SIGTRAP)
        {
            fprintf(stderr,"Bad status returned by waitpid: %#x\n",status);
            goto error;
        }

        test(pid,0,0xffffe000);
        test(pid,1,0xffffe000-1);

        kill(pid,SIGKILL);
        return 0;
      }
    }
error:
    kill(pid,SIGKILL);
    return 1;
}
// ------ end test program -------------

Regards,
Ruslan

[toc] | [next] | [standalone]

#1397675 — Re: Is it really correct to check for breakpoint in kernel space against ptracer's address space?

On 05/09/2016 01:43 AM, Ruslan Kabatsayev wrote:
> Hello all.
>
> Currently a 32-bit ptracer can't set HW breakpoints in tracee over
> address space limitations of _tracer_. Even if the tracee is 64-bit,
> doing PTRACE_POKEUSER into u_debugreg[n] with value>=0xffffe000 leads
> to EINVAL (below is a test tracer program to reproduce this). At the
> same time, if tracer is 64-bit, then for both 32- and 64-bit tracees
> the PTRACE_POKEUSER call will succeed even if violates address space
> constraints for tracee.
>
> I've traced this to arch_check_bp_in_kernel_space() in
> arch/x86/kernel/hw_breakpoint.c, which checks the address against
> TASK_SIZE, which as I understood refers to the current task, i.e.
> caller of the syscall, instead of the tracee (at least tracing this in
> Bochs leads me to this conclusion).

Is there any reason at all for TASK_SIZE to be different from TASK_SIZE_MAX?

/me needs to audit this stuff.

--Andy

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

#1398021 — Re: Is it really correct to check for breakpoint in kernel space against ptracer's address space?

On Tue, May 10, 2016 at 6:33 AM, Andy Lutomirski <luto@kernel.org> wrote:
> Is there any reason at all for TASK_SIZE to be different from TASK_SIZE_MAX?

The distinction originally appears to have been introduced by commit 8492980.

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

#1398245 — Re: Is it really correct to check for breakpoint in kernel space against ptracer's address space?

On Tue, May 10, 2016 at 4:50 AM, Ruslan Kabatsayev
<b7.10110111@gmail.com> wrote:
> On Tue, May 10, 2016 at 6:33 AM, Andy Lutomirski <luto@kernel.org> wrote:
>> Is there any reason at all for TASK_SIZE to be different from TASK_SIZE_MAX?
>
> The distinction originally appears to have been introduced by commit 8492980.

Hmm.

Anyway, what kernel are you on?  I think I fixed your specific issue in:

commit 27747f8bc355a2808ca9e490ab6866acd85b4c16
Author: Andy Lutomirski <luto@kernel.org>
Date:   Thu Jul 30 20:32:42 2015 -0700

    perf/x86/hw_breakpoints: Fix check for kernel-space breakpoints

If that does fix it and it's a problem for you on older kernels, you
could ask for a backport.

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC

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

#1398262 — Re: Is it really correct to check for breakpoint in kernel space against ptracer's address space?

On Tue, May 10, 2016 at 6:23 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> Anyway, what kernel are you on?  I think I fixed your specific issue in:
>
> commit 27747f8bc355a2808ca9e490ab6866acd85b4c16
> Author: Andy Lutomirski <luto@kernel.org>
> Date:   Thu Jul 30 20:32:42 2015 -0700
>
>     perf/x86/hw_breakpoints: Fix check for kernel-space breakpoints
>
> If that does fix it and it's a problem for you on older kernels, you
> could ask for a backport.

Indeed, I was using a 3.12 kernel. This should indeed fix it, thanks.

BTW, in that commit, why isn't va>=TASK_SIZE_MAX redundant with the
second operand of ||? If va is aligned to a multiple of len (so
va+len-1 doesn't overflow), then in what cases would the first
condition be true with the second being false?

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

#1398297 — Re: Is it really correct to check for breakpoint in kernel space against ptracer's address space?

On Tue, May 10, 2016 at 8:40 AM, Ruslan Kabatsayev
<b7.10110111@gmail.com> wrote:
> On Tue, May 10, 2016 at 6:23 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> Anyway, what kernel are you on?  I think I fixed your specific issue in:
>>
>> commit 27747f8bc355a2808ca9e490ab6866acd85b4c16
>> Author: Andy Lutomirski <luto@kernel.org>
>> Date:   Thu Jul 30 20:32:42 2015 -0700
>>
>>     perf/x86/hw_breakpoints: Fix check for kernel-space breakpoints
>>
>> If that does fix it and it's a problem for you on older kernels, you
>> could ask for a backport.
>
> Indeed, I was using a 3.12 kernel. This should indeed fix it, thanks.
>
> BTW, in that commit, why isn't va>=TASK_SIZE_MAX redundant with the
> second operand of ||? If va is aligned to a multiple of len (so
> va+len-1 doesn't overflow), then in what cases would the first
> condition be true with the second being false?

An excellent question :)

I think that, when I wrote that, I wasn't 100% convinced that len
couldn't be zero.

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web