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

* Is it really correct to check for breakpoint in kernel space against ptracer's address space?
@ 2016-05-09  8:43 Ruslan Kabatsayev
  2016-05-10  3:33 ` Andy Lutomirski
  0 siblings, 1 reply; 6+ messages in thread
From: Ruslan Kabatsayev @ 2016-05-09  8:43 UTC (permalink / raw)
  To: linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread