From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1755211AbdKIOrn (ORCPT <rfc822;w@1wt.eu>);
        Thu, 9 Nov 2017 09:47:43 -0500
Received: from mx1.redhat.com ([209.132.183.28]:29312 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1755188AbdKIOrl (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 9 Nov 2017 09:47:41 -0500
Date: Thu, 9 Nov 2017 15:47:38 +0100
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
Message-ID: <20171109144738.GA12923@redhat.com>
References: <20171109080155.359718-1-yhs@fb.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20171109080155.359718-1-yhs@fb.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Thu, 09 Nov 2017 14:47:41 +0000 (UTC)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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.