Received-SPF: pass (google.com: domain of linux-kernel+bounces-134770-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Mon, 8 Apr 2024 12:54:01 +0900
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Jiri Olsa <olsajiri@gmail.com>, Andrii Nakryiko
 <andrii.nakryiko@gmail.com>, Steven Rostedt <rostedt@goodmis.org>, Alexei
 Starovoitov <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net>,
 Andrii Nakryiko <andrii@kernel.org>, linux-kernel@vger.kernel.org,
 linux-trace-kernel@vger.kernel.org, bpf@vger.kernel.org, Song Liu
 <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>, John Fastabend
 <john.fastabend@gmail.com>, Peter Zijlstra <peterz@infradead.org>, Thomas
 Gleixner <tglx@linutronix.de>, "Borislav Petkov (AMD)" <bp@alien8.de>,
 x86@kernel.org, linux-api@vger.kernel.org
Subject: Re: [PATCHv2 1/3] uprobe: Add uretprobe syscall to speed up return
 probe
Message-Id: <20240408125401.d4f100d184b11bc01fcd0308@kernel.org>
In-Reply-To: <20240406175558.GC3060@redhat.com>
References: <20240403230937.c3bd47ee47c102cd89713ee8@kernel.org>
	<CAEf4BzZ2RFfz8PNgJ4ENZ0us4uX=DWhYFimXdtWms-VvGXOjgQ@mail.gmail.com>
	<20240404095829.ec5db177f29cd29e849169fa@kernel.org>
	<CAEf4BzYH60TwvBipHWB_kUqZZ6D-iUVnnFsBv06imRikK3o-bg@mail.gmail.com>
	<20240405005405.9bcbe5072d2f32967501edb3@kernel.org>
	<20240404161108.GG7153@redhat.com>
	<20240405102203.825c4a2e9d1c2be5b2bffe96@kernel.org>
	<Zg-8r63tPSkuhN7p@krava>
	<20240405110230.GA22839@redhat.com>
	<20240406120536.57374198f3f45e809d7e4efa@kernel.org>
	<20240406175558.GC3060@redhat.com>
Precedence: bulk
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Sat, 6 Apr 2024 19:55:59 +0200
Oleg Nesterov <oleg@redhat.com> wrote:

> On 04/06, Masami Hiramatsu wrote:
> >
> > On Fri, 5 Apr 2024 13:02:30 +0200
> > Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > > With or without this patch userpace can also do
> > >
> > > 	foo() { <-- retprobe1
> > > 		bar() {
> > > 			jump to xol_area
> > > 		}
> > > 	}
> > >
> > > handle_trampoline() will handle retprobe1.
> >
> > This is OK because the execution path has been changed to trampoline,
> 
> Agreed, in this case the misuse is more clear. But please see below.
> 
> > but the above will continue running bar() after sys_uretprobe().
> 
> .. and most probably crash

Yes, unless it returns with longjmp(). (but this is rare case and
maybe malicious program.)

> 
> > > sigreturn() can be "improved" too. Say, it could validate sigcontext->ip
> > > and return -EINVAL if this addr is not valid. But why?
> >
> > Because sigreturn() never returns, but sys_uretprobe() will return.
> 
> You mean, sys_uretprobe() returns to the next insn after syscall.
> 
> Almost certainly yes, but this is not necessarily true. If one of consumers
> changes regs->sp sys_uretprobe() "returns" to another location, just like
> sys_rt_sigreturn().
> 
> That said.
> 
> Masami, it is not that I am trying to prove that you are "wrong" ;) No.
> 
> I see your points even if I am biased, I understand that my objections are
> not 100% "fair".
> 
> I am just trying to explain why, rightly or not, I care much less about the
> abuse of sys_uretprobe().

I would like to clear that the abuse of this syscall will not possible to harm
the normal programs, and even if it is used by malicious code (e.g. injected by
stack overflow) it doesn't cause a problem. At least thsese points are cleared,
and documented. it is easier to push it as new Linux API.

Thank you,

> 
> Thanks!
> 
> Oleg.
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>