linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: tixy@yxit.co.uk (Tixy)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] kprobes/arm: fix emulation of LDR/STR instruction when Rn == PC
Date: Fri, 25 Mar 2011 21:19:04 +0000	[thread overview]
Message-ID: <1301087944.2744.85.camel@computer2.home> (raw)
In-Reply-To: <1301072519-27937-1-git-send-email-viktor.rosendahl@nokia.com>

On Fri, 2011-03-25 at 19:01 +0200, Viktor Rosendahl wrote:
> The Rn value from the emulation is unconditionally written back; this is fine
> as long as Rn != PC because in that case, even if the instruction isn't a write
> back instruction, it will only result in the same value being written back.
> 
> In case Rn == PC, then the emulated instruction doesn't have the actual PC
> value in Rn but an adjusted value; when this is written back, it will result in
> the PC being incorrectly updated.

It looks like we have the same problem with emulate_ldrd and
emulate_strd as well.

> An altenative solution would be to check bits 24 and 22 to see whether the
> instruction actually is a write back instruction or not. I think it's
> enough to check whether Rn != PC,  because:
> - it's looks cheaper than the alternative
> - to my understaning it's not permitted to update the PC with a write back
> instruction, so we don't lose any ability to emulate legal instructions.
> - in case of writing back for non write back instructions where Rn != PC, it
> doesn't matter because the values are the same.

I agree that the check for Rn != PC seems simplest and sufficient.

> Regarding the second point above, it would possibly be prudent to add some
> checking to prep_emulate_ldr_str(), so that instructions with write back and
> Rn == PC would be rejected.

I don't think it is worth adding code to check for illegal instructions.
The toolchain shouldn't generate them in the first place, and there are
many places in the kprobe code which doesn't bother checking; there are
even comments like "may be invalid, don't care".

[...]

I'm currently working on implementing Thumb support in kprobes and am
writing test code as part of that. I planned on adding test cases for
ARM so hopefully will catch a few more instruction emulation bugs (if
there are any to be found).

-- 
Tixy

  reply	other threads:[~2011-03-25 21:19 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-25 17:01 [PATCH] kprobes/arm: fix emulation of LDR/STR instruction when Rn == PC Viktor Rosendahl
2011-03-25 21:19 ` Tixy [this message]
2011-03-28 15:56   ` [PATCH] Fix ldrd/strd emulation for kprobes/ARM Viktor Rosendahl
2011-03-28 22:39     ` Nicolas Pitre
2011-03-29 11:26       ` Viktor Rosendahl
2011-03-29 16:55         ` Nicolas Pitre
2011-03-29 18:31           ` Russell King - ARM Linux
2011-03-29 18:44             ` Nicolas Pitre
2011-03-30 13:42               ` [PATCH] Reject kprobes when Rn==15 and writeback is set Viktor Rosendahl
2011-03-30 15:52                 ` Tixy
2011-03-30 16:46                   ` Viktor Rosendahl
2011-03-30 17:20                     ` Tixy
2011-03-30 17:59                   ` Nicolas Pitre
2011-03-30 19:39                     ` Tixy
2011-03-30 20:48                       ` Nicolas Pitre
2011-03-30 14:09           ` [PATCH] Fix ldrd/strd emulation for kprobes/ARM Viktor Rosendahl
2011-03-29 12:55     ` Tixy
2011-03-29 13:46       ` Viktor Rosendahl
2011-03-29 14:03         ` Tixy
2011-03-29 17:07       ` Nicolas Pitre
2011-03-28 16:27   ` [PATCH] kprobes/arm: fix emulation of LDR/STR instruction when Rn == PC Viktor Rosendahl
2011-03-29  9:12     ` Tixy
2011-03-26  2:03 ` Nicolas Pitre

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=1301087944.2744.85.camel@computer2.home \
    --to=tixy@yxit.co.uk \
    --cc=linux-arm-kernel@lists.infradead.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).