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 S1755866AbdKNQDc (ORCPT <rfc822;w@1wt.eu>);
        Tue, 14 Nov 2017 11:03:32 -0500
Received: from mx1.redhat.com ([209.132.183.28]:60774 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1755821AbdKNQDH (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 14 Nov 2017 11:03:07 -0500
Date: Tue, 14 Nov 2017 17:03:04 +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][v3] uprobes/x86: emulate push insns for uprobe on x86
Message-ID: <20171114160304.GA19323@redhat.com>
References: <20171113221139.1516536-1-yhs@fb.com>
 <20171114155124.GB17667@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20171114155124.GB17667@redhat.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.38]); Tue, 14 Nov 2017 16:03:07 +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/14, Oleg Nesterov wrote:
>
> > +#ifdef CONFIG_X86_64
> > +	if (test_thread_flag(TIF_ADDR32))
> > +		return -ENOSYS;
> > +#endif
>
> No, this doesn't look right, see my previous email. You should do this
> check in the "if (insn->length == 2)" branch below, "push bp" should be
> emulated correctly.
>
> And test_thread_flag(TIF_ADDR32) is not right too. The caller is not
> necessarily the probed task. See is_64bit_mm(mm) in arch_uprobe_analyze_insn().
>
> And again... please check if uprobe_init_insn() fails or not in this case
> (32bit task does, say, "push r8"). If it fails, your V2 should be fine.
>
>
> To remind, uprobes && 32-bit is broken, let me quote my another email:
>
> 	The 3rd bug means that you simply can't uprobe a 32bit task on a 64bit
> 	system, the in_compat_syscall() logic in get_unmapped_area() looks very
> 	wrong although I need to re-check.

Yes,

> I didn't have time for this problem so far. But emulation should work, so
> you can hopefully test your patch.

Ah, no, sizeof_long() is broken by the same reason, so you can't test it...

OK, I'll try to do something tomorrow, then we will see what can we do
with your patch...

But it would be nice if you can check what uprobe_init_insn() does in this
case, see above.

Oleg.