All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Yonghong Song <yhs@fb.com>
Cc: mingo@kernel.org, tglx@linutronix.de, peterz@infradead.org,
	linux-kernel@vger.kernel.org, x86@kernel.org,
	netdev@vger.kernel.org, ast@fb.com, kernel-team@fb.com
Subject: Re: [PATCH] uprobes/x86: emulate push insns for uprobe on x86
Date: Thu, 9 Nov 2017 15:47:38 +0100	[thread overview]
Message-ID: <20171109144738.GA12923@redhat.com> (raw)
In-Reply-To: <20171109080155.359718-1-yhs@fb.com>

On 11/09, Yonghong Song wrote:
>
> +	if (insn_class == UPROBE_PUSH_INSN) {
> +		src_ptr = get_push_reg_ptr(auprobe, regs);
> +		reg_width = sizeof_long();
> +		sp = regs->sp;
> +		if (copy_to_user((void __user *)(sp - reg_width), src_ptr, reg_width))
> +			return false;
> +
> +		regs->sp = sp - reg_width;
> +		regs->ip += 1 + (auprobe->push.rex_prefix != 0);
> +		return true;

Another nit... You can rename push_ret_address() and use it here

		src_ptr = ...;
		if (push_ret_address(regs, *src_ptr))
			return false;

		regs->ip += ...;
		return true;

and I think get_push_reg_ptr() should just return "unsigned long", not the
pointer.

And again, please make a separate method for this code. Let me repeat, the
main reason for branch_xol_ops/etc is that we simply can not execute these
insns out-of-line, we have to emulate them. "push" differs, the only reason
why we may want to emulate it is optimization.

Oleg.

  parent reply	other threads:[~2017-11-09 14:47 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-09  8:01 [PATCH] uprobes/x86: emulate push insns for uprobe on x86 Yonghong Song
2017-11-09 13:44 ` Oleg Nesterov
2017-11-09 14:04   ` Oleg Nesterov
2017-11-09 21:53   ` Yonghong Song
2017-11-09 14:47 ` Oleg Nesterov [this message]
  -- strict thread matches above, loose matches on Subject: below --
2017-11-09 23:02 Yonghong Song
2017-11-10 13:06 ` Oleg Nesterov

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=20171109144738.GA12923@redhat.com \
    --to=oleg@redhat.com \
    --cc=ast@fb.com \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=yhs@fb.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.