Hi!
Le jeudi 19 octobre 2023, 18:07:08 EEST Masami Hiramatsu a écrit :
On Thu, 19 Oct 2023 09:51:04 -0400
Steven Rostedt rostedt@goodmis.org wrote:
On Thu, 19 Oct 2023 21:18:43 +0900
Masami Hiramatsu (Google) mhiramat@kernel.org wrote:
So why is this adding stable? (and as Greg's form letter states, that's not how you do that)
I don't see this as a fix but a new feature.
I asked him to make this a fix since the current kprobe event' behavior is somewhat strange. It puts the probe on only the "first symbol" if user specifies a symbol name which has multiple instances. In this case, the actual probe address can not be solved by name. User must specify the probe address by unique name + offset. Unless, it can put a probe on unexpected address, especially if it specifies non-unique symbol + offset, the address may NOT be the instruction boundary. To avoid this issue, it should check the given symbol is unique.
OK, so what is broken is that when you add a probe to a function that has multiple names, it will attach to the first one and not necessarily the one you want.
The change log needs to be more explicit in what the "bug" is. It does state this in a round about way, but it is written in a way that it doesn't stand out.
Previously to this commit, if func matches several symbols, a kprobe, being either sysfs or PMU, would only be installed for the first matching address. This could lead to some misunderstanding when some BPF code was never called because it was attached to a function which was indeed not called, because the effectively called one has no kprobes attached.So, this commit returns EADDRNOTAVAIL when func matches several symbols. This way, user needs to use address to remove the ambiguity.
What it should say is: When a kprobe is attached to a function that's name is not unique (is static and shares the name with other functions in the kernel), the kprobe is attached to the first function it finds. This is a bug as the function that it is attaching to is not necessarily the one that the user wants to attach to. Instead of blindly picking a function to attach to what is ambiguous, error with EADDRNOTAVAIL to let the user know that this function is not unique, and that the user must use another unique function with an address offset to get to the function they want to attach to.
Great!, yes this looks good to me too.
And yes, it should have:
Cc: stable@vger.kernel.org
which is how to mark something for stable, and
Fixes: ...
To the commit that caused the bug.
Yes, this should be the first one.
Fixes: 413d37d1eb69 ("tracing: Add kprobe-based event tracer")
Thank you! I should have thought about it nonetheless but I will take more care in the future!
Thank you,
-- Steve
Best regards.