linux-mips.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: James Hogan <james.hogan@imgtec.com>
To: "Maciej W. Rozycki" <macro@imgtec.com>
Cc: Marcin Nowakowski <marcin.nowakowski@imgtec.com>,
	Ralf Baechle <ralf@linux-mips.org>,
	linux-mips@linux-mips.org
Subject: Re: [PATCH v2 1/2] MIPS: ptrace: disallow setting watchpoints in kernel address space
Date: Tue, 24 Jan 2017 18:54:52 +0000	[thread overview]
Message-ID: <20170124185452.GL29015@jhogan-linux.le.imgtec.org> (raw)
Message-ID: <20170124185452.60ZYG128j892UgmnJNr45ujRStvAVIT7onpPsxI1-oo@z> (raw)
In-Reply-To: <alpine.DEB.2.00.1701232258380.13564@tp.orcam.me.uk>

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

Hi Maciej,

On Tue, Jan 24, 2017 at 05:09:32PM +0000, Maciej W. Rozycki wrote:
> On Mon, 23 Jan 2017, Marcin Nowakowski wrote:
> 
> > With certain EVA configurations it is possible for the kernel address
> > space to overlap user address space, which allows the user to set
> > watchpoints on kernel addresses via ptrace.
> > 
> > If a watchpoint is set in the watch exception handling code (after
> > exception level has been cleared) then the system will hang in an
> > infinite loop when hitting a watchpoint while trying to process it.
> > 
> > To prevent that simply disallow placing any watchpoints at addresses
> > above start of kernel that overlap userspace.
> 
>  This can be severely crippling for user debugging.  Is there no better 
> way?

See the v1 patches, but as you say below, an architectural fix is vastly
preferable. Other proposed solutions have so far added complexity and
fragility, while struggling to completely plug the original problem.

> 
>  Can't for example the low-level exception handling entry/exit code be 
> moved out of the way of the EVA overlap range and then all watchpoints 
> masked for the duration of kernel mode execution?  This would be quite 
> expensive, however it could only be executed if a task flag indicates 
> watchpoints are being used.

That doesn't cover data watches. RAM would still need accessing, e.g. to
save/restore the watch state from the thread context, or even to read
the task flag, and stack accesses in C code. The only safe way for it to
work would be to somehow disable or inhibit watchpoints before clearing
EXL, and re-enable them after setting EXL, though you'd still get a loop
of deferred watchpoints if it hit on the way out to user mode unless
cleared at the last moment before ERET.

> Alternatively perhaps we could clobber 
> CP0.EntryHi.ASID, at least temporarily; that would be cheaper.

Kernel mode still needs to access the user address space.

Alternatively we could set WatchHi.ASID to a reserved one, and only
clear/set the WatchHi.G bit (to bypass the ASID match) at the first/last
moment while EXL=1. It still wouldn't protect against code watches
around there exposing the kernel address of that code by the resulting
hang though, so would need to move the ebase out of the overlap range
too (which would have to be platform specific).

Cheers
James

> 
>  Overall I think this situation is asking for a watchpoint flag to be 
> added to inhibit hits in the kernel mode in hardware; for completeness 
> this probably actually ought to be a field to cover the kernel, supervisor 
> and user modes separately -- either a plain bitmask for arbitrary control 
> or an encoded value similar to CP0.Status.KSU which would indicate the 
> most privileged mode to accept a watchpoint in.
> 
>  I had a recollection of such a facility already being available for JTAG 
> debugging, but I can't track it down in the specification, so perhaps it 
> was for another architecture and it would be completely new for ours.
> 
>   Maciej

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

  parent reply	other threads:[~2017-01-24 18:54 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-23  9:18 [PATCH v2 1/2] MIPS: ptrace: disallow setting watchpoints in kernel address space Marcin Nowakowski
2017-01-23  9:18 ` Marcin Nowakowski
2017-01-23  9:18 ` [PATCH v2 2/2] MIPS: ptrace: disable watchpoints if hit in kernel mode Marcin Nowakowski
2017-01-23  9:18   ` Marcin Nowakowski
2017-01-24 17:09 ` [PATCH v2 1/2] MIPS: ptrace: disallow setting watchpoints in kernel address space Maciej W. Rozycki
2017-01-24 17:09   ` Maciej W. Rozycki
2017-01-24 18:54   ` James Hogan [this message]
2017-01-24 18:54     ` James Hogan
2017-01-24 20:52     ` Maciej W. Rozycki
2017-01-24 20:52       ` Maciej W. Rozycki
2017-01-24 22:05       ` James Hogan
2017-01-24 22:05         ` James Hogan
2017-01-24 23:07         ` Maciej W. Rozycki
2017-01-24 23:07           ` Maciej W. Rozycki
2017-01-25 14:39           ` Maciej W. Rozycki
2017-01-25 14:39             ` Maciej W. Rozycki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170124185452.GL29015@jhogan-linux.le.imgtec.org \
    --to=james.hogan@imgtec.com \
    --cc=linux-mips@linux-mips.org \
    --cc=macro@imgtec.com \
    --cc=marcin.nowakowski@imgtec.com \
    --cc=ralf@linux-mips.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).