On Thu, 19 Oct 2023 21:18:43 +0900
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> 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.
And yes, it should have:
Cc: stable@xxxxxxxxxxxxxxx
which is how to mark something for stable, and
Fixes: ...
To the commit that caused the bug.
-- Steve
