linux-kernel - Re: [PATCH 0/8] uprobes: RCU-protected hot path optimizations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4BzZ8SaFK4iMtPPxYZQjHOvaPqpKApE8=Bz+h29xq+xMEsA@mail.gmail.com>
Date: Wed, 7 Aug 2024 10:31:25 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Oleg Nesterov <oleg@...hat.com>, Liao Chang <liaochang1@...wei.com>
Cc: Andrii Nakryiko <andrii@...nel.org>, linux-trace-kernel@...r.kernel.org, 
	peterz@...radead.org, rostedt@...dmis.org, mhiramat@...nel.org, 
	bpf@...r.kernel.org, linux-kernel@...r.kernel.org, jolsa@...nel.org, 
	paulmck@...nel.org
Subject: Re: [PATCH 0/8] uprobes: RCU-protected hot path optimizations

On Wed, Aug 7, 2024 at 10:11 AM Oleg Nesterov <oleg@...hat.com> wrote:
>
> On 08/07, Andrii Nakryiko wrote:
> >
> > Yes, I was waiting for more of Peter's comments, but I guess I'll just
> > send a v2 today.
>
> OK,
>
> > I'll probably include the SRCU+timeout logic for
> > return_instances, and maybe lockless VMA parts as well.
>
> Well, feel free to do what you think right, but perhaps it would be
> better to push this series first? at least 1-4.

Ok, I can send those first 4 patches first and hopefully we can land
them soon and move to the next part. I just also wrote up details
about that crash in rb_find_rcu().

>
> As for lockless VMA. To me this needs more discussions. I didn't read

We are still discussing, feel free to join the conversation.

> your conversation with Peter and Suren carefully, but I too have some
> concerns. Most probably I am wrong, and until I saw this thread I didn't
> even know that vm_area_free() uses call_rcu() if CONFIG_PER_VMA_LOCK,
> but still.
>
> > > As for 8/8 - I leave it to you and Peter. I'd prefer SRCU though ;)
> >
> > Honestly curious, why the preference?
>
> Well, you can safely ignore me, but since you have asked ;)
>
> I understand what SRCU does, and years ago I even understood (I hope)
> the implementation. More or less the same for rcu_tasks. But as for
> the _trace flavour, I simply fail to understand its semantics.

Ok, I won't try to repeat Paul's explanations. If you are curious you
can find them in comments to my previous batch register/unregister API
patches.

>
> > BTW, while you are here :) What can you say about
> > current->sighand->siglock use in handle_singlestep()?
>
> It should die, and this looks simple. I disagree with the patches
> from Liao, see the
> https://lore.kernel.org/all/20240801082407.1618451-1-liaochang1@huawei.com/
> thread, but I agree with the intent.

I wasn't aware of this patch, thanks for mentioning it. Strange that
me or at least bpf@...r.kernel.org wasn't CC'ed.

Liao, please cc bpf@ mailing list for future patches like that.

>
> IMO, we need a simple "bool restore_sigpending" in uprobe_task, it will make the
> necessary changes really simple.

The simpler the better, I can't comment on correctness as I don't
understand the logic well enough. Are you going to send a patch with
your bool flag proposal?

>
> (To clarify. In fact I think that a new TIF_ or even PF_ flag makes more sense,
>  afaics it can have more users. But I don't think that uprobes can provide enough
>  justification for that right now)
>
> Oleg.
>