linux-kernel - Re: [PATCH 2/2] hw-breakpoint: Keep track of dr7 local enable bits

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c62985531002190749o250f7598gde0ab14cff4cce9a@mail.gmail.com>
Date:	Fri, 19 Feb 2010 16:49:53 +0100
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	prasad@...ux.vnet.ibm.com
Cc:	Ingo Molnar <mingo@...e.hu>, LKML <linux-kernel@...r.kernel.org>,
	Michael Stefaniuc <mstefani@...hat.com>,
	Alan Stern <stern@...land.harvard.edu>,
	Maneesh Soni <maneesh@...ux.vnet.ibm.com>,
	Alexandre Julliard <julliard@...ehq.org>,
	"Rafael J . Wysocki" <rjw@...k.pl>,
	Maciej Rutecki <maciej.rutecki@...il.com>,
	Roland McGrath <roland@...hat.com>
Subject: Re: [PATCH 2/2] hw-breakpoint: Keep track of dr7 local enable bits

2010/2/19 K.Prasad <prasad@...ux.vnet.ibm.com>:
> On Thu, Feb 18, 2010 at 07:00:01PM +0100, Frederic Weisbecker wrote:
>
>> diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
>> index 017d937..0c1033d 100644
>> --- a/arch/x86/kernel/ptrace.c
>> +++ b/arch/x86/kernel/ptrace.c
>> @@ -702,7 +702,7 @@ static unsigned long ptrace_get_debugreg(struct task_struct *tsk, int n)
>>       } else if (n == 6) {
>>               val = thread->debugreg6;
>>        } else if (n == 7) {
>> -             val = ptrace_get_dr7(thread->ptrace_bps);
>> +             val = thread->ptrace_dr7;
>
> Some more comments that I missed out in the previous mail...
>
> Shouldn't ptrace_get_dr7() function be entirely removed now, given that
> its only caller no longer invokes it?


Not in this set. This is an urgent fix, we shouldn't take corner risks
so late in the process.

Also, this thread->ptrace_dr7 is a temporary thing, a hack to fix a
regression without
taking too much risk. But we need something more proper in the longer term, like
storing this information in the arch_hw_breakpoint (we shouldn't bloat
the thread
structure with such mostly unused field).

I have some ideas to get the arch_hw_breakpoint more "arch", to
register breakpoints
using arch informations only, so that we don't depend on a generic
breakpoint attribute
conversion for ptrace, which may be too limited for tricky arch
breakpoint implementations,
and an unecessary midlayer there.
I'll tell you more while answering to your ppc breakpoint implementation.



>>       }
>>       return val;
>>  }
>> @@ -778,8 +778,11 @@ int ptrace_set_debugreg(struct task_struct *tsk, int n, unsigned long val)
>>                       return rc;
>>       }
>>       /* All that's left is DR7 */
>> -     if (n == 7)
>> +     if (n == 7) {
>>               rc = ptrace_write_dr7(tsk, val);
>
> And ptrace_write_dr7() should be made to populate thread->ptrace_dr7 if
> it is going to return a success.


Yeah we can do that from ptrace_write_dr7(). Either way.
It's just more quick to do that here. I did not think much
about it as I don't think about ptrace_dr7 as a long term field.

Thanks.


>> +             if (!rc)
>> +                     thread->ptrace_dr7 = val;
>> +     }
>>
>>  ret_path:
>>       return rc;
--
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/