Linux Trace Kernel
 help / color / mirror / Atom feed
* [PATCH v4 2/2] selftests/ftrace: Add new test case which checks non unique symbol
From: Francis Laniel @ 2023-08-25 14:05 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, linux-trace-kernel, Francis Laniel,
	Steven Rostedt, Shuah Khan, linux-kselftest
In-Reply-To: <20230825140519.34297-1-flaniel@linux.microsoft.com>

If name_show() is non unique, this test will try to install a kprobe on this
function which should fail returning EADDRNOTAVAIL.
On kernel where name_show() is not unique, this test is skipped.

Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
---
 .../ftrace/test.d/kprobe/kprobe_non_uniq_symbol.tc  | 13 +++++++++++++
 1 file changed, 13 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_non_uniq_symbol.tc

diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_non_uniq_symbol.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_non_uniq_symbol.tc
new file mode 100644
index 000000000000..bc9514428dba
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_non_uniq_symbol.tc
@@ -0,0 +1,13 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: Test failure of registering kprobe on non unique symbol
+# requires: kprobe_events
+
+SYMBOL='name_show'
+
+# We skip this test on kernel where SYMBOL is unique or does not exist.
+if [ "$(grep -c -E "[[:alnum:]]+ t ${SYMBOL}" /proc/kallsyms)" -le '1' ]; then
+	exit_unsupported
+fi
+
+! echo "p:test_non_unique ${SYMBOL}" > kprobe_events
-- 
2.34.1


^ permalink raw reply related

* [PATCH v4 1/2] tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols
From: Francis Laniel @ 2023-08-25 14:05 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, linux-trace-kernel, Francis Laniel,
	Steven Rostedt
In-Reply-To: <20230825140519.34297-1-flaniel@linux.microsoft.com>

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.

Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
Link: https://lore.kernel.org/lkml/20230819101105.b0c104ae4494a7d1f2eea742@kernel.org/
---
 kernel/trace/trace_kprobe.c | 63 +++++++++++++++++++++++++++++++++++++
 kernel/trace/trace_probe.h  |  1 +
 2 files changed, 64 insertions(+)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 23dba01831f7..92dbb21c6961 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -705,6 +705,25 @@ static struct notifier_block trace_kprobe_module_nb = {
 	.priority = 1	/* Invoked after kprobe module callback */
 };
 
+static int count_symbols(void *data, unsigned long unused)
+{
+	unsigned int *count = data;
+
+	(*count)++;
+
+	return 0;
+}
+
+static unsigned int number_of_same_symbols(char *func_name)
+{
+	unsigned int count;
+
+	count = 0;
+	kallsyms_on_each_match_symbol(count_symbols, func_name, &count);
+
+	return count;
+}
+
 static int __trace_kprobe_create(int argc, const char *argv[])
 {
 	/*
@@ -836,6 +855,31 @@ static int __trace_kprobe_create(int argc, const char *argv[])
 		}
 	}
 
+	if (symbol && !strchr(symbol, ':')) {
+		unsigned int count;
+
+		count = number_of_same_symbols(symbol);
+		if (count > 1) {
+			/*
+			 * Users should use ADDR to remove the ambiguity of
+			 * using KSYM only.
+			 */
+			trace_probe_log_err(0, NON_UNIQ_SYMBOL);
+			ret = -EADDRNOTAVAIL;
+
+			goto error;
+		} else if (count == 0) {
+			/*
+			 * We can return ENOENT earlier than when register the
+			 * kprobe.
+			 */
+			trace_probe_log_err(0, BAD_PROBE_ADDR);
+			ret = -ENOENT;
+
+			goto error;
+		}
+	}
+
 	trace_probe_log_set_index(0);
 	if (event) {
 		ret = traceprobe_parse_event_name(&event, &group, gbuf,
@@ -1699,6 +1743,7 @@ static int unregister_kprobe_event(struct trace_kprobe *tk)
 }
 
 #ifdef CONFIG_PERF_EVENTS
+
 /* create a trace_kprobe, but don't add it to global lists */
 struct trace_event_call *
 create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
@@ -1709,6 +1754,24 @@ create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
 	int ret;
 	char *event;
 
+	if (func) {
+		unsigned int count;
+
+		count = number_of_same_symbols(func);
+		if (count > 1)
+			/*
+			 * Users should use addr to remove the ambiguity of
+			 * using func only.
+			 */
+			return ERR_PTR(-EADDRNOTAVAIL);
+		else if (count == 0)
+			/*
+			 * We can return ENOENT earlier than when register the
+			 * kprobe.
+			 */
+			return ERR_PTR(-ENOENT);
+	}
+
 	/*
 	 * local trace_kprobes are not added to dyn_event, so they are never
 	 * searched in find_trace_kprobe(). Therefore, there is no concern of
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 01ea148723de..975161164366 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -438,6 +438,7 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call,
 	C(BAD_MAXACT,		"Invalid maxactive number"),		\
 	C(MAXACT_TOO_BIG,	"Maxactive is too big"),		\
 	C(BAD_PROBE_ADDR,	"Invalid probed address or symbol"),	\
+	C(NON_UNIQ_SYMBOL,	"The symbol is not unique"),		\
 	C(BAD_RETPROBE,		"Retprobe address must be an function entry"), \
 	C(NO_TRACEPOINT,	"Tracepoint is not found"),		\
 	C(BAD_ADDR_SUFFIX,	"Invalid probed address suffix"), \
-- 
2.34.1


^ permalink raw reply related

* [PATCH v4 0/2] Return EADDRNOTAVAIL when func matches several symbols during kprobe creation
From: Francis Laniel @ 2023-08-25 14:05 UTC (permalink / raw)
  To: linux-kernel; +Cc: Masami Hiramatsu, linux-trace-kernel, Francis Laniel

Hi.


In the kernel source code, it exists different functions which share the same
name but which have, of course, different addresses as they can be defined in
different modules:
# Kernel was compiled with CONFIG_NTFS_FS and CONFIG_NTFS3_FS as built-in.
root@vm-amd64:~# grep ntfs_file_write_iter /proc/kallsyms
ffffffff814ce3c0 t __pfx_ntfs_file_write_iter
ffffffff814ce3d0 t ntfs_file_write_iter
ffffffff814fc8a0 t __pfx_ntfs_file_write_iter
ffffffff814fc8b0 t ntfs_file_write_iter
This can be source of troubles when you create a PMU kprobe for such a function,
as it will only install one for the first address (e.g. 0xffffffff814ce3d0 in
the above).
This could lead to some troubles were BPF based tools does not report any event
because the second function is not called:
root@vm-amd64:/mnt# mount | grep /mnt
/foo.img on /mnt type ntfs3 (rw,relatime,uid=0,gid=0,iocharset=utf8)
# ig is a tool which installs a PMU kprobe on ntfs_file_write_iter().
root@vm-amd64:/mnt# ig trace fsslower -m 0 -f ntfs3 --host &> /tmp/foo &
[1] 207
root@vm-amd64:/mnt# dd if=./foo of=./bar count=3
3+0 records in
3+0 records out
1536 bytes (1.5 kB, 1.5 KiB) copied, 0.00543323 s, 283 kB/s
root@vm-amd64:/mnt# fg
ig trace fsslower -m 0 -f ntfs3 --host &> /tmp/foo
^Croot@vm-amd64:/mnt# more /tmp/foo
RUNTIME.CONTAINERNAME          RUNTIME.CONTAIN… PID              COMM
  T      BYTES     OFFSET        LAT FILE
                                                214              dd
  R        512          0        766 foo
                                                214              dd
  R        512        512          9 foo
                                                214              dd
As you can see in the above, only read events are reported and no write because
the kprobe is installed for the old ntfs_file_write_iter() and not the ntfs3
one.
The same behavior occurs with sysfs kprobe:
root@vm-amd64:/# echo 'p:probe/ntfs_file_write_iter ntfs_file_write_iter' > /sys/kernel/tracing/kprobe_events
root@vm-amd64:/# cat /sys/kernel/tracing/kprobe_events
p:probe/ntfs_file_write_iter ntfs_file_write_iter
root@vm-amd64:/# mount | grep /mnt
/foo.img on /mnt type ntfs3 (rw,relatime,uid=0,gid=0,iocharset=utf8)
root@vm-amd64:/# perf record -e probe:ntfs_file_write_iter &
[1] 210
root@vm-amd64:/# cd /mnt/
root@vm-amd64:/mnt# dd if=./foo of=./bar count=3
3+0 records in
3+0 records out
1536 bytes (1.5 kB, 1.5 KiB) copied, 0.00234793 s, 654 kB/s
root@vm-amd64:/mnt# cd -
/
root@vm-amd64:/# fg
perf record -e probe:ntfs_file_write_iter
^C[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.056 MB perf.data ]

root@vm-amd64:/# perf report
Error:
The perf.data data has no samples!
# To display the perf.data header info, please use --header/--header-only optio>
#

In this contribution, I modified the functions creating sysfs and PMU kprobes to
test if the function name given as argument matches several symbols.
In this case, these functions return EADDRNOTAVAIL to indicate the user to use
addr and offs to remove this ambiguity.
So, when the above BPF tool is run, the following error message is printed:
root@vm-amd64:~# ig trace fsslower -m 0 -f ntfs3 --host &> /tmp/foo &
[1] 228
root@vm-amd64:~# more /tmp/foo
RUNTIME.CONTAINERNAME          RUNTIME.CONTAIN… PID              COMM
  T      BYTES     OFFSET        LAT FILE
Error: running gadget: running gadget: installing tracer: attaching kprobe: crea
ting perf_kprobe PMU (arch-specific fallback for "ntfs_file_write_iter"): token
ntfs_file_write_iter: opening perf event: cannot assign requested address
And the same with sysfs kprobe:
root@vm-amd64:/# echo 'p:probe/ntfs_file_write_iter ntfs_file_write_iter' > /sys/kernel/tracing/kprobe_events
-bash: echo: write error: Cannot assign requested address
Note that, this does not influence perf as it installs kprobes as offset on
_text:
root@vm-amd64:/# perf probe --add ntfs_file_write_iter
Added new events:
  probe:ntfs_file_write_iter (on ntfs_file_write_iter)
  probe:ntfs_file_write_iter (on ntfs_file_write_iter)
...
root@vm-amd64:/# cat /sys/kernel/tracing/kprobe_events
p:probe/ntfs_file_write_iter _text+5039088
p:probe/ntfs_file_write_iter _text+5228752

Note that, this contribution is the conclusion of a previous RFC which intended
to install a PMU kprobe for all matching symbols [1, 2].

If you see any way to improve this contribution, particularly if you have an
idea to add tests or documentation for this behavior, please share your
feedback.

Changes since:
 v1:
  * Use EADDRNOTAVAIL instead of adding a new error code.
  * Correct also this behavior for sysfs kprobe.
 v2:
  * Count the number of symbols corresponding to function name and return
  EADDRNOTAVAIL if higher than 1.
  * Return ENOENT if above count is 0, as it would be returned later by while
  registering the kprobe.
 v3:
  * Check symbol does not contain ':' before testing its uniqueness.
  * Add a selftest to check this is not possible to install a kprobe for a non
  unique symbol.

Francis Laniel (2):
  tracing/kprobes: Return EADDRNOTAVAIL when func matches several
    symbols
  selftests/ftrace: Add new test case which checks non unique symbol

 kernel/trace/trace_kprobe.c                   | 63 +++++++++++++++++++
 kernel/trace/trace_probe.h                    |  1 +
 .../test.d/kprobe/kprobe_non_uniq_symbol.tc   | 13 ++++
 3 files changed, 77 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_non_uniq_symbol.tc


Best regards and thank you in advance.
---
[1]: https://lore.kernel.org/lkml/20230816163517.112518-1-flaniel@linux.microsoft.com/
[2]: https://lore.kernel.org/lkml/20230819101105.b0c104ae4494a7d1f2eea742@kernel.org/
--
2.34.1


^ permalink raw reply

* [PATCH v4] tracepoint: add new `tcp:tcp_ca_event` trace event
From: Zheao Li @ 2023-08-25 13:32 UTC (permalink / raw)
  To: edumazet, mhiramat, rostedt, davem, dsahern, kuba, pabeni
  Cc: netdev, linux-kernel, linux-trace-kernel, bpf, Zheao Li

Hello 

This the 4th version of the patch, the previous discusstion is here

https://lore.kernel.org/linux-trace-kernel/20230807183308.9015-1-me@manjusaka.me/

In this version of the code, here's some different:

1. The event name has been changed from `tcp_ca_event_set` to
`tcp_ca_event`

2. Output the current protocol family in TP_printk

3. Show the ca_event symbol instead of the original number

But the `./scripts/checkpatch.pl` has been failed to check this patch,
because we sill have some code error in ./scripts/checkpatch.pl(in
another world, the test would be failed when we use the 
scripts/checkpatch.pl to check the events/tcp.h

Feel free to ask me if you have have any issues and ideas.

Thanks

---

In normal use case, the tcp_ca_event would be changed in high frequency.

The developer can monitor the network quality more easier by tracing
TCP stack with this TP event.

So I propose to add a `tcp:tcp_ca_event` trace event
like `tcp:tcp_cong_state_set` to help the people to
trace the TCP connection status

Signed-off-by: Zheao Li <me@manjusaka.me>
---
 include/net/tcp.h          |  9 ++----
 include/trace/events/tcp.h | 60 ++++++++++++++++++++++++++++++++++++++
 net/ipv4/tcp_cong.c        | 10 +++++++
 3 files changed, 72 insertions(+), 7 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 0ca972ebd3dd..a68c5b61889c 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1154,13 +1154,8 @@ static inline bool tcp_ca_needs_ecn(const struct sock *sk)
 	return icsk->icsk_ca_ops->flags & TCP_CONG_NEEDS_ECN;
 }
 
-static inline void tcp_ca_event(struct sock *sk, const enum tcp_ca_event event)
-{
-	const struct inet_connection_sock *icsk = inet_csk(sk);
-
-	if (icsk->icsk_ca_ops->cwnd_event)
-		icsk->icsk_ca_ops->cwnd_event(sk, event);
-}
+/* from tcp_cong.c */
+void tcp_ca_event(struct sock *sk, const enum tcp_ca_event event);
 
 /* From tcp_cong.c */
 void tcp_set_ca_state(struct sock *sk, const u8 ca_state);
diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
index 7b1ddffa3dfc..993eb00403ea 100644
--- a/include/trace/events/tcp.h
+++ b/include/trace/events/tcp.h
@@ -41,6 +41,18 @@
 	TP_STORE_V4MAPPED(__entry, saddr, daddr)
 #endif
 
+/* The TCP CA event traced by tcp_ca_event*/
+#define tcp_ca_event_names    \
+		EM(CA_EVENT_TX_START)     \
+		EM(CA_EVENT_CWND_RESTART) \
+		EM(CA_EVENT_COMPLETE_CWR) \
+		EM(CA_EVENT_LOSS)         \
+		EM(CA_EVENT_ECN_NO_CE)    \
+		EMe(CA_EVENT_ECN_IS_CE)
+
+#define show_tcp_ca_event_names(val) \
+	__print_symbolic(val, tcp_ca_event_names)
+
 /*
  * tcp event with arguments sk and skb
  *
@@ -419,6 +431,54 @@ TRACE_EVENT(tcp_cong_state_set,
 		  __entry->cong_state)
 );
 
+TRACE_EVENT(tcp_ca_event,
+
+	TP_PROTO(struct sock *sk, const u8 ca_event),
+
+	TP_ARGS(sk, ca_event),
+
+	TP_STRUCT__entry(
+		__field(const void *, skaddr)
+		__field(__u16, sport)
+		__field(__u16, dport)
+		__field(__u16, family)
+		__array(__u8, saddr, 4)
+		__array(__u8, daddr, 4)
+		__array(__u8, saddr_v6, 16)
+		__array(__u8, daddr_v6, 16)
+		__field(__u8, ca_event)
+	),
+
+	TP_fast_assign(
+		struct inet_sock *inet = inet_sk(sk);
+		__be32 *p32;
+
+		__entry->skaddr = sk;
+
+		__entry->sport = ntohs(inet->inet_sport);
+		__entry->dport = ntohs(inet->inet_dport);
+		__entry->family = sk->sk_family;
+
+		p32 = (__be32 *) __entry->saddr;
+		*p32 = inet->inet_saddr;
+
+		p32 = (__be32 *) __entry->daddr;
+		*p32 =  inet->inet_daddr;
+
+		TP_STORE_ADDRS(__entry, inet->inet_saddr, inet->inet_daddr,
+			   sk->sk_v6_rcv_saddr, sk->sk_v6_daddr);
+
+		__entry->ca_event = ca_event;
+	),
+
+	TP_printk("family=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c ca_event=%s",
+		  show_family_name(__entry->family),
+		  __entry->sport, __entry->dport,
+		  __entry->saddr, __entry->daddr,
+		  __entry->saddr_v6, __entry->daddr_v6,
+		  show_tcp_ca_event_names(__entry->ca_event))
+);
+
 #endif /* _TRACE_TCP_H */
 
 /* This part must be outside protection */
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
index 1b34050a7538..fb7ec6ebbbd0 100644
--- a/net/ipv4/tcp_cong.c
+++ b/net/ipv4/tcp_cong.c
@@ -34,6 +34,16 @@ struct tcp_congestion_ops *tcp_ca_find(const char *name)
 	return NULL;
 }
 
+void tcp_ca_event(struct sock *sk, const enum tcp_ca_event event)
+{
+	const struct inet_connection_sock *icsk = inet_csk(sk);
+
+	trace_tcp_ca_event(sk, (u8)event);
+
+	if (icsk->icsk_ca_ops->cwnd_event)
+		icsk->icsk_ca_ops->cwnd_event(sk, event);
+}
+
 void tcp_set_ca_state(struct sock *sk, const u8 ca_state)
 {
 	struct inet_connection_sock *icsk = inet_csk(sk);
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH -next v2] rv/include: set variable 'da_mon_##name' to static
From: Daniel Bristot de Oliveira @ 2023-08-25 13:25 UTC (permalink / raw)
  To: Yu Liao, rostedt; +Cc: linux-kernel, linux-trace-kernel, liwei391
In-Reply-To: <20230823020051.3184953-1-liaoyu15@huawei.com>

On 8/23/23 04:00, Yu Liao wrote:
> gcc with W=1 reports
> kernel/trace/rv/monitors/wip/wip.c:20:1: sparse: sparse: symbol 'da_mon_wip' was not declared. Should it be static?
> 
> The per-cpu variable 'da_mon_##name' is only used in its defining file, so
> it should be static.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202307280030.7EjUG9gR-lkp@intel.com/
> Signed-off-by: Yu Liao <liaoyu15@huawei.com>

Acked-by: Daniel Bristot de Oliveira <bristot@kernel.org>

Thanks!
-- Daniel

^ permalink raw reply

* Re: [PATCH v3 1/1] tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols
From: Masami Hiramatsu @ 2023-08-25 13:13 UTC (permalink / raw)
  To: Francis Laniel; +Cc: linux-kernel, linux-trace-kernel, Steven Rostedt
In-Reply-To: <5704161.DvuYhMxLoT@pwmachine>

On Fri, 25 Aug 2023 14:34:49 +0200
Francis Laniel <flaniel@linux.microsoft.com> wrote:

> Hi.
> 
> Le vendredi 25 août 2023, 14:16:49 CEST Masami Hiramatsu a écrit :
> > On Thu, 24 Aug 2023 18:08:59 +0200
> > 
> > Francis Laniel <flaniel@linux.microsoft.com> wrote:
> > > 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.
> > > 
> > > Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
> > > Link:
> > > https://lore.kernel.org/lkml/20230819101105.b0c104ae4494a7d1f2eea742@kern
> > > el.org/ ---
> > 
> > Ah, this should be fine, but selftest (tools/testing/selftests/ftrace)
> > fails.
> > 
> >  # tail 60-kprobe_module.tc-log.vsOHnF
> > ...
> > + :
> > + : 'Add an event on a module function without specifying event name'
> > + :
> > + echo 'p trace_printk:trace_printk_irq_work'
> > sh: write error: No such file or directory
> > 
> > Ah, the function on non-exist module should be checked too.
> > 
> > # tail 63-kprobe_syntax_errors.tc-log.mMLwIQ
> > + + printfwc '%s' -c
> >  'p '
> > + pos=2
> > + printf+  '%s'tr 'p ^non_exist_func'
> >  -d ^
> > + command='p non_exist_func'
> > + echo 'Test command: p non_exist_func'
> > Test command: p non_exist_func
> > + echo
> > + grep 'trace_kprobe: error:' -A 3 error_log
> > 
> > Also, this doesn't leave a syntax error message.
> > 
> > So, the below changes are needed.
> 
> Excellent catch! Thank you, I will apply this patch and send v4 right after.
> Regarding test, do you think I can add a test for the EADDRNOTAVAIL case?

Hmm, in that case, you need to change something in tracefs/README so that
we can identify the kernel has different behavior. Or we have to change
this is a "Fix" for backporting.

> Maybe it should go inside LTP? As this would need having a kernel compiled 
> with a name pointing to several symbols?

For this tracing feature, I rather like to use tools/testing/selftests/ftrace
to test it. And it is used on all stable kernel, that is why we need to add
some change on tracefs/README or something.

But I would like to wait for Alessandro's work. After his work, in this time
we need to probe all the same-name symbols as your original patch does.
This is because 1:n mapping can happen as Alessandro pointed in

https://lore.kernel.org/all/CAPp5cGQsRdB0+KHR1wX2bDDdc5sTzSNPA417PNJb0ypmV=yS6w@mail.gmail.com/

But if his feature is configurable (and maybe so), we need to keep this
version... We have many options.

- this normal kallsyms: the same-name symbols should not be used.
- enhanced kallsyms (if 1:n symbol has the same suffix): the same name symbols
   should be probed at once.
- enhanced kallsyms (if 1:n symbol has different suffix): the same-name symbol
   must not exist.


> 
> Also, should some man pages somewhere be updated to reflect the case kprobe can 
> return EADDRNOTAVAIL?

No, it is a tracefs interface and we don't have man pages yet.

Thank you,

> 
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index 8ab46a2a446d..1e57bc896952 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -855,7 +855,7 @@ static int __trace_kprobe_create(int argc, const char
> > *argv[]) }
> >  	}
> > 
> > -	if (symbol) {
> > +	if (symbol && !strchr(symbol, ':')) {
> >  		unsigned int count;
> > 
> >  		count = number_of_same_symbols(symbol);
> > @@ -864,6 +864,7 @@ static int __trace_kprobe_create(int argc, const char
> > *argv[]) * Users should use ADDR to remove the ambiguity of
> >  			 * using KSYM only.
> >  			 */
> > +			trace_probe_log_err(0, NON_UNIQ_SYMBOL);
> >  			ret = -EADDRNOTAVAIL;
> > 
> >  			goto error;
> > @@ -872,6 +873,7 @@ static int __trace_kprobe_create(int argc, const char
> > *argv[]) * We can return ENOENT earlier than when register the
> >  			 * kprobe.
> >  			 */
> > +			trace_probe_log_err(0, BAD_PROBE_ADDR);
> >  			ret = -ENOENT;
> > 
> >  			goto error;
> > diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> > index 7f929482e8d4..a4f478448eef 100644
> > --- a/kernel/trace/trace_probe.h
> > +++ b/kernel/trace/trace_probe.h
> > @@ -450,6 +450,7 @@ extern int traceprobe_define_arg_fields(struct
> > trace_event_call *event_call, C(BAD_MAXACT,		"Invalid maxactive
> > number"),		\
> >  	C(MAXACT_TOO_BIG,	"Maxactive is too big"),		\
> >  	C(BAD_PROBE_ADDR,	"Invalid probed address or symbol"),	\
> > +	C(NON_UNIQ_SYMBOL,	"The symbol is not unique"),		\
> >  	C(BAD_RETPROBE,		"Retprobe address must be an function 
> entry"), \
> >  	C(NO_TRACEPOINT,	"Tracepoint is not found"),		\
> >  	C(BAD_ADDR_SUFFIX,	"Invalid probed address suffix"), \
> > 
> > Thank you,
> > 
> > >  kernel/trace/trace_kprobe.c | 61 +++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 61 insertions(+)
> > > 
> > > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > > index 23dba01831f7..2f393739e8cf 100644
> > > --- a/kernel/trace/trace_kprobe.c
> > > +++ b/kernel/trace/trace_kprobe.c
> > > @@ -705,6 +705,25 @@ static struct notifier_block trace_kprobe_module_nb =
> > > {> 
> > >  	.priority = 1	/* Invoked after kprobe module callback */
> > >  
> > >  };
> > > 
> > > +static int count_symbols(void *data, unsigned long unused)
> > > +{
> > > +	unsigned int *count = data;
> > > +
> > > +	(*count)++;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static unsigned int number_of_same_symbols(char *func_name)
> > > +{
> > > +	unsigned int count;
> > > +
> > > +	count = 0;
> > > +	kallsyms_on_each_match_symbol(count_symbols, func_name, &count);
> > > +
> > > +	return count;
> > > +}
> > > +
> > > 
> > >  static int __trace_kprobe_create(int argc, const char *argv[])
> > >  {
> > >  
> > >  	/*
> > > 
> > > @@ -836,6 +855,29 @@ static int __trace_kprobe_create(int argc, const char
> > > *argv[])> 
> > >  		}
> > >  	
> > >  	}
> > > 
> > > +	if (symbol) {
> > > +		unsigned int count;
> > > +
> > > +		count = number_of_same_symbols(symbol);
> > > +		if (count > 1) {
> > > +			/*
> > > +			 * Users should use ADDR to remove the ambiguity of
> > > +			 * using KSYM only.
> > > +			 */
> > > 
> > > 
> > > 
> > > +			ret = -EADDRNOTAVAIL;
> > > +
> > > +			goto error;
> > > +		} else if (count == 0) {
> > > +			/*
> > > +			 * We can return ENOENT earlier than when register the
> > > +			 * kprobe.
> > > +			 */
> > > +			ret = -ENOENT;
> > > +
> > > +			goto error;
> > > +		}
> > > +	}
> > > +
> > > 
> > >  	trace_probe_log_set_index(0);
> > >  	if (event) {
> > >  	
> > >  		ret = traceprobe_parse_event_name(&event, &group, gbuf,
> > > 
> > > @@ -1699,6 +1741,7 @@ static int unregister_kprobe_event(struct
> > > trace_kprobe *tk)> 
> > >  }
> > >  
> > >  #ifdef CONFIG_PERF_EVENTS
> > > 
> > > +
> > > 
> > >  /* create a trace_kprobe, but don't add it to global lists */
> > >  struct trace_event_call *
> > >  create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
> > > 
> > > @@ -1709,6 +1752,24 @@ create_local_trace_kprobe(char *func, void *addr,
> > > unsigned long offs,> 
> > >  	int ret;
> > >  	char *event;
> > > 
> > > +	if (func) {
> > > +		unsigned int count;
> > > +
> > > +		count = number_of_same_symbols(func);
> > > +		if (count > 1)
> > > +			/*
> > > +			 * Users should use addr to remove the ambiguity of
> > > +			 * using func only.
> > > +			 */
> > > +			return ERR_PTR(-EADDRNOTAVAIL);
> > > +		else if (count == 0)
> > > +			/*
> > > +			 * We can return ENOENT earlier than when register the
> > > +			 * kprobe.
> > > +			 */
> > > +			return ERR_PTR(-ENOENT);
> > > +	}
> > > +
> > > 
> > >  	/*
> > >  	
> > >  	 * local trace_kprobes are not added to dyn_event, so they are never
> > >  	 * searched in find_trace_kprobe(). Therefore, there is no concern of
> 
> Best regards.
> 
> 


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

^ permalink raw reply

* Re: [PATCH] trace/hwlat: remove extra space at the end of hwlat_detector/mode
From: Daniel Bristot de Oliveira @ 2023-08-25 12:53 UTC (permalink / raw)
  To: Mikhail Kobuk, Steven Rostedt
  Cc: Masami Hiramatsu, linux-kernel, linux-trace-kernel, lvc-project,
	Alexey Khoroshilov
In-Reply-To: <20230825103432.7750-1-m.kobuk@ispras.ru>

On 8/25/23 12:34, Mikhail Kobuk wrote:
> Space is printed after each mode value including the last one:
> $ echo \"$(sudo cat /sys/kernel/tracing/hwlat_detector/mode)\"
> "none [round-robin] per-cpu "
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: 8fa826b7344d ("trace/hwlat: Implement the mode config option")
> Signed-off-by: Mikhail Kobuk <m.kobuk@ispras.ru>
> Reviewed-by: Alexey Khoroshilov <khoroshilov@ispras.ru>

Acked-by: Daniel Bristot de Oliveira <bristot@kernel.org>

Thanks!
-- Daniel

^ permalink raw reply

* Re: [PATCH v3 1/1] tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols
From: Francis Laniel @ 2023-08-25 12:34 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: linux-kernel, Masami Hiramatsu, linux-trace-kernel,
	Steven Rostedt
In-Reply-To: <20230825211649.188a81321f00297ec161a046@kernel.org>

Hi.

Le vendredi 25 août 2023, 14:16:49 CEST Masami Hiramatsu a écrit :
> On Thu, 24 Aug 2023 18:08:59 +0200
> 
> Francis Laniel <flaniel@linux.microsoft.com> wrote:
> > 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.
> > 
> > Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
> > Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
> > Link:
> > https://lore.kernel.org/lkml/20230819101105.b0c104ae4494a7d1f2eea742@kern
> > el.org/ ---
> 
> Ah, this should be fine, but selftest (tools/testing/selftests/ftrace)
> fails.
> 
>  # tail 60-kprobe_module.tc-log.vsOHnF
> ...
> + :
> + : 'Add an event on a module function without specifying event name'
> + :
> + echo 'p trace_printk:trace_printk_irq_work'
> sh: write error: No such file or directory
> 
> Ah, the function on non-exist module should be checked too.
> 
> # tail 63-kprobe_syntax_errors.tc-log.mMLwIQ
> + + printfwc '%s' -c
>  'p '
> + pos=2
> + printf+  '%s'tr 'p ^non_exist_func'
>  -d ^
> + command='p non_exist_func'
> + echo 'Test command: p non_exist_func'
> Test command: p non_exist_func
> + echo
> + grep 'trace_kprobe: error:' -A 3 error_log
> 
> Also, this doesn't leave a syntax error message.
> 
> So, the below changes are needed.

Excellent catch! Thank you, I will apply this patch and send v4 right after.
Regarding test, do you think I can add a test for the EADDRNOTAVAIL case?
Maybe it should go inside LTP? As this would need having a kernel compiled 
with a name pointing to several symbols?

Also, should some man pages somewhere be updated to reflect the case kprobe can 
return EADDRNOTAVAIL?

> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 8ab46a2a446d..1e57bc896952 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -855,7 +855,7 @@ static int __trace_kprobe_create(int argc, const char
> *argv[]) }
>  	}
> 
> -	if (symbol) {
> +	if (symbol && !strchr(symbol, ':')) {
>  		unsigned int count;
> 
>  		count = number_of_same_symbols(symbol);
> @@ -864,6 +864,7 @@ static int __trace_kprobe_create(int argc, const char
> *argv[]) * Users should use ADDR to remove the ambiguity of
>  			 * using KSYM only.
>  			 */
> +			trace_probe_log_err(0, NON_UNIQ_SYMBOL);
>  			ret = -EADDRNOTAVAIL;
> 
>  			goto error;
> @@ -872,6 +873,7 @@ static int __trace_kprobe_create(int argc, const char
> *argv[]) * We can return ENOENT earlier than when register the
>  			 * kprobe.
>  			 */
> +			trace_probe_log_err(0, BAD_PROBE_ADDR);
>  			ret = -ENOENT;
> 
>  			goto error;
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 7f929482e8d4..a4f478448eef 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -450,6 +450,7 @@ extern int traceprobe_define_arg_fields(struct
> trace_event_call *event_call, C(BAD_MAXACT,		"Invalid maxactive
> number"),		\
>  	C(MAXACT_TOO_BIG,	"Maxactive is too big"),		\
>  	C(BAD_PROBE_ADDR,	"Invalid probed address or symbol"),	\
> +	C(NON_UNIQ_SYMBOL,	"The symbol is not unique"),		\
>  	C(BAD_RETPROBE,		"Retprobe address must be an function 
entry"), \
>  	C(NO_TRACEPOINT,	"Tracepoint is not found"),		\
>  	C(BAD_ADDR_SUFFIX,	"Invalid probed address suffix"), \
> 
> Thank you,
> 
> >  kernel/trace/trace_kprobe.c | 61 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 61 insertions(+)
> > 
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index 23dba01831f7..2f393739e8cf 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -705,6 +705,25 @@ static struct notifier_block trace_kprobe_module_nb =
> > {> 
> >  	.priority = 1	/* Invoked after kprobe module callback */
> >  
> >  };
> > 
> > +static int count_symbols(void *data, unsigned long unused)
> > +{
> > +	unsigned int *count = data;
> > +
> > +	(*count)++;
> > +
> > +	return 0;
> > +}
> > +
> > +static unsigned int number_of_same_symbols(char *func_name)
> > +{
> > +	unsigned int count;
> > +
> > +	count = 0;
> > +	kallsyms_on_each_match_symbol(count_symbols, func_name, &count);
> > +
> > +	return count;
> > +}
> > +
> > 
> >  static int __trace_kprobe_create(int argc, const char *argv[])
> >  {
> >  
> >  	/*
> > 
> > @@ -836,6 +855,29 @@ static int __trace_kprobe_create(int argc, const char
> > *argv[])> 
> >  		}
> >  	
> >  	}
> > 
> > +	if (symbol) {
> > +		unsigned int count;
> > +
> > +		count = number_of_same_symbols(symbol);
> > +		if (count > 1) {
> > +			/*
> > +			 * Users should use ADDR to remove the ambiguity of
> > +			 * using KSYM only.
> > +			 */
> > 
> > 
> > 
> > +			ret = -EADDRNOTAVAIL;
> > +
> > +			goto error;
> > +		} else if (count == 0) {
> > +			/*
> > +			 * We can return ENOENT earlier than when register the
> > +			 * kprobe.
> > +			 */
> > +			ret = -ENOENT;
> > +
> > +			goto error;
> > +		}
> > +	}
> > +
> > 
> >  	trace_probe_log_set_index(0);
> >  	if (event) {
> >  	
> >  		ret = traceprobe_parse_event_name(&event, &group, gbuf,
> > 
> > @@ -1699,6 +1741,7 @@ static int unregister_kprobe_event(struct
> > trace_kprobe *tk)> 
> >  }
> >  
> >  #ifdef CONFIG_PERF_EVENTS
> > 
> > +
> > 
> >  /* create a trace_kprobe, but don't add it to global lists */
> >  struct trace_event_call *
> >  create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
> > 
> > @@ -1709,6 +1752,24 @@ create_local_trace_kprobe(char *func, void *addr,
> > unsigned long offs,> 
> >  	int ret;
> >  	char *event;
> > 
> > +	if (func) {
> > +		unsigned int count;
> > +
> > +		count = number_of_same_symbols(func);
> > +		if (count > 1)
> > +			/*
> > +			 * Users should use addr to remove the ambiguity of
> > +			 * using func only.
> > +			 */
> > +			return ERR_PTR(-EADDRNOTAVAIL);
> > +		else if (count == 0)
> > +			/*
> > +			 * We can return ENOENT earlier than when register the
> > +			 * kprobe.
> > +			 */
> > +			return ERR_PTR(-ENOENT);
> > +	}
> > +
> > 
> >  	/*
> >  	
> >  	 * local trace_kprobes are not added to dyn_event, so they are never
> >  	 * searched in find_trace_kprobe(). Therefore, there is no concern of

Best regards.



^ permalink raw reply

* Re: [PATCH v2] scripts/link-vmlinux.sh: Add alias to duplicate symbols for kallsyms
From: Masami Hiramatsu @ 2023-08-25 12:31 UTC (permalink / raw)
  To: Alessandro Carminati
  Cc: Francis Laniel, Steven Rostedt, Alexander Lobakin, Nick Alcock,
	Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, Masami Hiramatsu, Daniel Bristot de Oliveira,
	Viktor Malik, Kris Van Hees, Luis Chamberlain, eugene.loh,
	Josh Poimboeuf, linux-kernel, linux-kbuild, live-patching,
	linux-trace-kernel
In-Reply-To: <CAPp5cGQsRdB0+KHR1wX2bDDdc5sTzSNPA417PNJb0ypmV=yS6w@mail.gmail.com>

Hi Alessandro,

On Fri, 25 Aug 2023 12:15:09 +0200
Alessandro Carminati <alessandro.carminati@gmail.com> wrote:

> > Thank you for the investigation, this an interesting reading!
> >
> > > The rationale is that using source file + line number iproduces better
> > > kallsyms table, but it is still do not produce unique names.
> >
> > When I first read your cover letter, I also thought it would be cool to have
> > the filename rather than a serial ID.
> > So, your latest modification is really good as it goes into this direction.
> > Regarding having non unique names, I am not sure if this is a problem.
> > In my case, which is using kprobe to attach BPF code to trace some events,
> > your contribution would make me on the way to know which address I should use
> > because I could choose regarding the filename.
> 
> This whole thing got trickier than I expected when I started. At first, I was
> just grappling with different functions having the same name.
> But during my investigation, it hit me that I've got to handle multiple copies
> of the same function too.
> So, those addresses labeled with the same name?
> They're not just different functions having the same name - they're a mix of
> functions that happen to share a name and functions that are the same but come
> from different compile units.
> 
> Imagine this:
> one compile unit defines a function foo1() that uses a bar() from a
> shared header.
> Another compile unit defines foo2(), also relying on bar().
> The snag is, these compile units end up being linked together, carrying their
> own copies of bar().

Ah, there are 1:n mapping issue along with n:1 issue. In that case, both symbols
should be probed. ('perf probe' adds probes for all those instances, usually
happens on probing inlined function)

> So, if I wanna keep an eye on bar(), which copy should I watch?
> The answer depends on whose bar() I want to watch at.
> I could go for probing them all, sure, but that's kind of a suboptimal
> solution that waters down what I'm trying to achieve.

I think we can keep the same symbols in the kallsyms in that case, because
those may have the same function prototype. (.constprop suffix has different
type but we can keep .constprope suffix to the symbols in that case)
If your feature ensures that the same-name symbols has the same type, we
can probe all same-name symbols with the same BPF programs or probe-event.

Again, I think the important point is the same name symbols has the same
prototype.

> > I am curious about a potential v3 of this patch!
> 
> The way things are being done right now, even though it's nice that it can
> track down these copies in the kernel image, it doesn't quite hit the
> bullseye of what I was going for. But hey, I'm totally open to putting out
> v3 with the limitations I just talked about, and discuss if it can be improved.

Thank you for your work!

> 
> >
> > > BR
> > > Alessandro
> > >
> > > > As a final note, please understand that my patch was not intended to
> > > > undermine anyone's work. I simply encountered a problem that I wanted to
> > > > help solve. Attached to this message is my code, in case anyone wants to
> > > > replicate it. I would appreciate being kept in the loop, as I genuinely
> > > > want to assist in fixing this problem.
> > > >
> > > > BR
> > > > Alessandro
> > > > ---
> > > >
> > > >  init/Kconfig                        |  36 ++++
> > > >  scripts/Makefile                    |   4 +
> > > >  scripts/kas_alias/Makefile          |   4 +
> > > >  scripts/kas_alias/a2l.c             | 268 ++++++++++++++++++++++++++++
> > > >  scripts/kas_alias/a2l.h             |  32 ++++
> > > >  scripts/kas_alias/duplicates_list.c |  70 ++++++++
> > > >  scripts/kas_alias/duplicates_list.h |  15 ++
> > > >  scripts/kas_alias/item_list.c       | 230 ++++++++++++++++++++++++
> > > >  scripts/kas_alias/item_list.h       |  26 +++
> > > >  scripts/kas_alias/kas_alias.c       | 217 ++++++++++++++++++++++
> > > >  scripts/link-vmlinux.sh             |  11 +-
> > > >  11 files changed, 910 insertions(+), 3 deletions(-)
> > > >  create mode 100644 scripts/kas_alias/Makefile
> > > >  create mode 100644 scripts/kas_alias/a2l.c
> > > >  create mode 100644 scripts/kas_alias/a2l.h
> > > >  create mode 100644 scripts/kas_alias/duplicates_list.c
> > > >  create mode 100644 scripts/kas_alias/duplicates_list.h
> > > >  create mode 100644 scripts/kas_alias/item_list.c
> > > >  create mode 100644 scripts/kas_alias/item_list.h
> > > >  create mode 100644 scripts/kas_alias/kas_alias.c
> > > >
> > > > diff --git a/init/Kconfig b/init/Kconfig
> > > > index f7f65af4ee12..bc69fcd9cbc8 100644
> > > > --- a/init/Kconfig
> > > > +++ b/init/Kconfig
> > > > @@ -1737,6 +1737,42 @@ config KALLSYMS_BASE_RELATIVE
> > > >
> > > >           time constants, and no relocation pass is required at runtime to
> > > >           fix
> > > >           up the entries based on the runtime load address of the kernel.
> > > >
> > > > +config KALLSYMS_ALIAS
> > > > +       bool "Produces alias for duplicated symbols" if EXPERT
> > > > +       depends on KALLSYMS && (DEBUG_INFO_DWARF4 || DEBUG_INFO_DWARF5)
> > > > +       help
> > > > +         It is not uncommon for drivers or modules related to similar
> > > > +         peripherals to have symbols with the exact same name.
> > > > +         While this is not a problem for the kernel's binary itself, it
> > > > +         becomes an issue when attempting to trace or probe specific
> > > > +         functions using infrastructure like ftrace or kprobe.
> > > > +
> > > > +         This option addresses this challenge by extending the symbol
> > > > names +         with unique suffixes during the kernel build process.
> > > > +         The newly created aliases for these duplicated symbols are
> > > > unique
> > > > +         names that can be fed to the ftrace sysfs interface. By doing
> > > > so, it +         enables previously unreachable symbols to be probed.
> > > > +
> > > > +config CONFIG_KALLSYMS_ALIAS_DATA
> > > > +       bool "Produces alias also for data"
> > > > +       depends on KALLSYMS_ALIAS
> > > > +       help
> > > > +         Sometimes it can be useful to refer to data. In live patch
> > > > scenarios, +         you may find yourself needing to use symbols that
> > > > are shared with +         other functions. Since symbols face the same
> > > > issue as functions, this +         option allows you to create aliases
> > > > for data as well.
> > > > +
> > > > +config CONFIG_KALLSYMS_ALIAS_DATA_ALL
> > > > +       bool "Removes all filter when producing data alias"
> > > > +       depends on CONFIG_KALLSYMS_ALIAS_DATA
> > > > +       help
> > > > +         When selecting data aliases, not all symbols are included in the
> > > > set +         This is because many symbols are unlikely to be used. If
> > > > you choose +         to have an alias for all data symbols, be aware that
> > > > it will +         significantly increase the size.
> > > > +
> > > > +         If unsure, say N.
> > > > +
> > > >
> > > >  # end of the "standard kernel features (expert users)" menu
> > > >
> > > >  # syscall, maps, verifier
> > > >
> > > > diff --git a/scripts/Makefile b/scripts/Makefile
> > > > index 32b6ba722728..65fafe17cfe5 100644
> > > > --- a/scripts/Makefile
> > > > +++ b/scripts/Makefile
> > > > @@ -49,3 +49,7 @@ subdir-$(CONFIG_SECURITY_SELINUX) += selinux
> > > >
> > > >  # Let clean descend into subdirs
> > > >  subdir-        += basic dtc gdb kconfig mod
> > > >
> > > > +
> > > > +# KALLSyms alias
> > > > +subdir-$(CONFIG_KALLSYMS_ALIAS) += kas_alias
> > > > +
> > > > diff --git a/scripts/kas_alias/Makefile b/scripts/kas_alias/Makefile
> > > > new file mode 100644
> > > > index 000000000000..e1fde69232b4
> > > > --- /dev/null
> > > > +++ b/scripts/kas_alias/Makefile
> > > > @@ -0,0 +1,4 @@
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +hostprogs-always-$(CONFIG_KALLSYMS_ALIAS)    += kas_alias
> > > > +
> > > > +kas_alias-objs        := duplicates_list.o item_list.o kas_alias.o a2l.o
> > > > diff --git a/scripts/kas_alias/a2l.c b/scripts/kas_alias/a2l.c
> > > > new file mode 100644
> > > > index 000000000000..a9692ac30180
> > > > --- /dev/null
> > > > +++ b/scripts/kas_alias/a2l.c
> > > > @@ -0,0 +1,268 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > > +#include <stdio.h>
> > > > +#include <stdlib.h>
> > > > +#include <string.h>
> > > > +#include <unistd.h>
> > > > +#include <sys/types.h>
> > > > +#include <sys/wait.h>
> > > > +#include <string.h>
> > > > +#include <stdint.h>
> > > > +#include <stdbool.h>
> > > > +
> > > > +#include "a2l.h"
> > > > +
> > > > +int addr2line_pid = -1;
> > > > +int a2l_in[2];
> > > > +int a2l_out[2];
> > > > +char line[MAX_BUF];
> > > > +char vmlinux_path[MAX_BUF];
> > > > +char addr2line_cmd[MAX_CMD_LEN];
> > > > +FILE *a2l_stdin, *a2l_stdout;
> > > > +
> > > > +static char *normalize_path(const char *input_path, char *output_path)
> > > > +{
> > > > +       char *prev_token = NULL;
> > > > +       char *delimiter = "/";
> > > > +       char inbuf[MAX_BUF];
> > > > +       char *token;
> > > > +       char *pos;
> > > > +
> > > > +       memset(inbuf, 0, MAX_BUF);
> > > > +       *output_path = '\0';
> > > > +       strncpy(inbuf, input_path, MAX_BUF);
> > > > +       if (!input_path || !output_path || strlen(input_path) == 0)
> > > > +               return NULL;
> > > > +
> > > > +       token = strtok(inbuf, delimiter);
> > > > +       while (token) {
> > > > +               if (strcmp(token, "..") == 0 && prev_token) {
> > > > +                       pos = strrchr(output_path, '/');
> > > > +                       if (pos)
> > > > +                               *pos = '\0';
> > > > +
> > > > +               } else if (strcmp(token, ".") != 0) {
> > > > +                       strcat(output_path, "/");
> > > > +                       strcat(output_path, token);
> > > > +               }
> > > > +
> > > > +               prev_token = token;
> > > > +               token = strtok(NULL, delimiter);
> > > > +       }
> > > > +
> > > > +       return output_path;
> > > > +}
> > > > +
> > > > +static void path_of(const char *full_path, char *path)
> > > > +{
> > > > +       const char *last_slash = strrchr(full_path, '/');
> > > > +       size_t path_length;
> > > > +       char cwd[MAX_BUF];
> > > > +
> > > > +       if (!last_slash) {
> > > > +               if (getcwd(cwd, sizeof(cwd)))
> > > > +                       strcpy(path, cwd);
> > > > +               else
> > > > +                       strcpy(path, ".");
> > > > +       } else {
> > > > +               path_length = last_slash - full_path;
> > > > +               strncpy(path, full_path, path_length);
> > > > +               path[path_length] = '\0';
> > > > +       }
> > > > +}
> > > > +
> > > > +static bool file_exists(const char *file_path)
> > > > +{
> > > > +       FILE *file;
> > > > +
> > > > +       file = fopen(file_path, "r");
> > > > +       if (file) {
> > > > +               fclose(file);
> > > > +               return true;
> > > > +       }
> > > > +       return false;
> > > > +}
> > > > +
> > > > +int addr2line_init(const char *cmd, const char *vmlinux)
> > > > +{
> > > > +       if ((!file_exists(cmd)) || (!file_exists(vmlinux))) {
> > > > +               printf("file not found\n");
> > > > +               return 0;
> > > > +               }
> > > > +
> > > > +       path_of(vmlinux, vmlinux_path);
> > > > +       if (pipe(a2l_in) == -1) {
> > > > +               printf("Failed to create pipe\n");
> > > > +               return 0;
> > > > +       }
> > > > +
> > > > +       if (pipe(a2l_out) == -1) {
> > > > +               printf("Failed to create pipe\n");
> > > > +               return 0;
> > > > +       }
> > > > +
> > > > +       addr2line_pid = fork();
> > > > +       if (addr2line_pid == -1) {
> > > > +               printf("Failed to fork process\n");
> > > > +               close(a2l_in[P_READ]);
> > > > +               close(a2l_in[P_WRITE]);
> > > > +               close(a2l_out[P_READ]);
> > > > +               close(a2l_out[P_WRITE]);
> > > > +               return 0;
> > > > +       }
> > > > +
> > > > +       if (addr2line_pid == 0) {
> > > > +               dup2(a2l_in[P_READ], 0);
> > > > +               dup2(a2l_out[P_WRITE], 1);
> > > > +               close(a2l_in[P_WRITE]);
> > > > +               close(a2l_out[P_READ]);
> > > > +
> > > > +               execlp(cmd, cmd, ADDR2LINE_ARGS, vmlinux, NULL);
> > > > +
> > > > +               printf("Failed to execute addr2line command\n");
> > > > +               exit(1);
> > > > +       } else {
> > > > +               close(a2l_in[P_READ]);
> > > > +               close(a2l_out[P_WRITE]);
> > > > +       }
> > > > +
> > > > +       a2l_stdin = fdopen(a2l_in[P_WRITE], "w");
> > > > +       if (!a2l_stdin) {
> > > > +               printf("Failed to open pipe a2l_in\n");
> > > > +               return 0;
> > > > +       }
> > > > +
> > > > +       a2l_stdout = fdopen(a2l_out[P_READ], "r");
> > > > +       if (!a2l_stdout) {
> > > > +               printf("Failed to open pipe a2l_out\n");
> > > > +               fclose(a2l_stdin);
> > > > +               return 0;
> > > > +       }
> > > > +
> > > > +       return 1;
> > > > +}
> > > > +
> > > > +const char *remove_subdir(const char *home, const char *f_path)
> > > > +{
> > > > +       int i = 0;
> > > > +
> > > > +       while (*(home + i) == *(f_path + i))
> > > > +               i++;
> > > > +
> > > > +       return (strlen(home) != i) ? NULL : f_path + i;
> > > > +}
> > > > +
> > > > +char *addr2line_get_lines(uint64_t address)
> > > > +{
> > > > +       char buf[MAX_BUF];
> > > > +
> > > > +       fprintf(a2l_stdin, "%08lx\n", address);
> > > > +       fflush(a2l_stdin);
> > > > +
> > > > +       if (!fgets(line, sizeof(line), a2l_stdout)) {
> > > > +               printf("Failed to read lines from addr2line\n");
> > > > +               return NULL;
> > > > +       }
> > > > +
> > > > +       if (!fgets(line, sizeof(line), a2l_stdout)) {
> > > > +               printf("Failed to read lines from addr2line\n");
> > > > +               return NULL;
> > > > +       }
> > > > +
> > > > +       line[strcspn(line, "\n")] = '\0';
> > > > +       strncpy(buf, line, MAX_BUF);
> > > > +       return normalize_path(buf, line);
> > > > +}
> > > > +
> > > > +int addr2line_cleanup(void)
> > > > +{
> > > > +       int status;
> > > > +
> > > > +       if (addr2line_pid != -1) {
> > > > +               kill(addr2line_pid, SIGKILL);
> > > > +               waitpid(addr2line_pid, &status, 0);
> > > > +               fclose(a2l_stdin);
> > > > +               fclose(a2l_stdout);
> > > > +               addr2line_pid = -1;
> > > > +       }
> > > > +
> > > > +       return 1;
> > > > +}
> > > > +
> > > > +static char *find_executable(const char *command)
> > > > +{
> > > > +       char *path_env = getenv("PATH");
> > > > +       char *executable_path;
> > > > +       char *path_copy;
> > > > +       char *path;
> > > > +       int n;
> > > > +
> > > > +       if (!path_env)
> > > > +               return NULL;
> > > > +
> > > > +       path_copy = strdup(path_env);
> > > > +       if (!path_copy)
> > > > +               return NULL;
> > > > +
> > > > +       path = strtok(path_copy, ":");
> > > > +       while (path) {
> > > > +               n = snprintf(0, 0, "%s/%s", path, command);
> > > > +               executable_path = (char *)malloc(n + 1);
> > > > +               snprintf(executable_path, n + 1, "%s/%s", path, command);
> > > > +               if (access(executable_path, X_OK) == 0) {
> > > > +                       free(path_copy);
> > > > +                       return executable_path;
> > > > +               }
> > > > +
> > > > +       path = strtok(NULL, ":");
> > > > +       free(executable_path);
> > > > +       executable_path = NULL;
> > > > +       }
> > > > +
> > > > +       free(path_copy);
> > > > +       if (executable_path)
> > > > +               free(executable_path);
> > > > +       return NULL;
> > > > +}
> > > > +
> > > > +const char *get_addr2line(int mode)
> > > > +{
> > > > +       char *buf = "";
> > > > +
> > > > +       switch (mode) {
> > > > +       case A2L_CROSS:
> > > > +               buf = getenv("CROSS_COMPILE");
> > > > +               memcpy(addr2line_cmd, buf, strlen(buf));
> > > > +       case A2L_DEFAULT:
> > > > +               memcpy(addr2line_cmd + strlen(buf), ADDR2LINE,
> > > > strlen(ADDR2LINE)); +               buf = find_executable(addr2line_cmd);
> > > > +               if (buf) {
> > > > +                       memcpy(addr2line_cmd, buf, strlen(buf));
> > > > +                       free(buf);
> > > > +               }
> > > > +               return addr2line_cmd;
> > > > +       case A2L_LLVM:
> > > > +       default:
> > > > +               return NULL;
> > > > +       }
> > > > +}
> > > > +
> > > > +char *get_vmlinux(char *input)
> > > > +{
> > > > +       const char *match_string1 = ".syms";
> > > > +       const char *match_string2 = ".tmp_vmlinux.kallsyms";
> > > > +       char *result = NULL;
> > > > +       char *match_pos;
> > > > +
> > > > +       match_pos = strstr(input, match_string1);
> > > > +       if (!match_pos)
> > > > +               return NULL;
> > > > +
> > > > +       match_pos = strstr(input, match_string2);
> > > > +       if (!match_pos)
> > > > +               return NULL;
> > > > +
> > > > +       result = strdup(input);
> > > > +       match_pos = strstr(result, match_string1);
> > > > +       *match_pos = '\0';
> > > > +       return result;
> > > > +}
> > > > diff --git a/scripts/kas_alias/a2l.h b/scripts/kas_alias/a2l.h
> > > > new file mode 100644
> > > > index 000000000000..ca6419229dde
> > > > --- /dev/null
> > > > +++ b/scripts/kas_alias/a2l.h
> > > > @@ -0,0 +1,32 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > > +#ifndef A2L_H
> > > > +#define A2L_H
> > > > +#include <stdint.h>
> > > > +
> > > > +#define ADDR2LINE "addr2line"
> > > > +#define ADDR2LINE_ARGS "-fe"
> > > > +//#define VMLINUX "vmlinux"
> > > > +#define MAX_BUF 4096
> > > > +#define MAX_CMD_LEN 256
> > > > +#define P_READ 0
> > > > +#define P_WRITE 1
> > > > +#define A2L_DEFAULT 1
> > > > +#define A2L_CROSS 2
> > > > +#define A2L_LLVM 3
> > > > +#define A2L_MAKE_VALUE 2
> > > > +
> > > > +extern int addr2line_pid;
> > > > +extern int a2l_in[2];
> > > > +extern int a2l_out[2];
> > > > +extern char line[MAX_BUF];
> > > > +extern char vmlinux_path[MAX_BUF];
> > > > +extern char addr2line_cmd[MAX_CMD_LEN];
> > > > +
> > > > +int addr2line_init(const char *cmd, const char *vmlinux);
> > > > +char *addr2line_get_lines(uint64_t address);
> > > > +int addr2line_cleanup(void);
> > > > +const char *remove_subdir(const char *home, const char *f_path);
> > > > +const char *get_addr2line(int mode);
> > > > +char *get_vmlinux(char *input);
> > > > +
> > > > +#endif
> > > > diff --git a/scripts/kas_alias/duplicates_list.c
> > > > b/scripts/kas_alias/duplicates_list.c new file mode 100644
> > > > index 000000000000..e7a3d2917937
> > > > --- /dev/null
> > > > +++ b/scripts/kas_alias/duplicates_list.c
> > > > @@ -0,0 +1,70 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > > +#include <stdint.h>
> > > > +#include <stdio.h>
> > > > +#include <string.h>
> > > > +#include <stdlib.h>
> > > > +#include <stdbool.h>
> > > > +
> > > > +#include "item_list.h"
> > > > +#include "duplicates_list.h"
> > > > +
> > > > +struct duplicate_item *find_duplicates(struct item *list)
> > > > +{
> > > > +       struct duplicate_item *current_duplicate = NULL;
> > > > +       struct duplicate_item *duplicates = NULL;
> > > > +       struct duplicate_item *new_duplicate;
> > > > +       struct item *current_item = list;
> > > > +       bool prev_was_duplicate = false;
> > > > +       struct item *prev_item = NULL;
> > > > +
> > > > +       while (current_item) {
> > > > +               if ((prev_item && (strcmp(current_item->symb_name,
> > > > prev_item->symb_name) == 0)) || +                   prev_was_duplicate) {
> > > > +                       if (!duplicates) {
> > > > +                               duplicates = malloc(sizeof(struct
> > > > duplicate_item)); +                               if (!duplicates)
> > > > +                                       return NULL;
> > > > +
> > > > +                               duplicates->original_item = prev_item;
> > > > +                               duplicates->next = NULL;
> > > > +                               current_duplicate = duplicates;
> > > > +                       } else {
> > > > +                               new_duplicate = malloc(sizeof(struct
> > > > duplicate_item)); +                               if (!new_duplicate) {
> > > > +                                       free_duplicates(&duplicates);
> > > > +                                       return NULL;
> > > > +                               }
> > > > +
> > > > +                               new_duplicate->original_item = prev_item;
> > > > +                               new_duplicate->next = NULL;
> > > > +                               current_duplicate->next = new_duplicate;
> > > > +                               current_duplicate = new_duplicate;
> > > > +
> > > > +                               if ((strcmp(current_item->symb_name,
> > > > prev_item->symb_name) != 0) && +
> > > > (prev_was_duplicate))
> > > > +                                       prev_was_duplicate = false;
> > > > +                               else
> > > > +                                       prev_was_duplicate = true;
> > > > +                       }
> > > > +               }
> > > > +
> > > > +               prev_item = current_item;
> > > > +               current_item = current_item->next;
> > > > +       }
> > > > +
> > > > +       return duplicates;
> > > > +}
> > > > +
> > > > +void free_duplicates(struct duplicate_item **duplicates)
> > > > +{
> > > > +       struct duplicate_item *duplicates_iterator = *duplicates;
> > > > +       struct duplicate_item *app;
> > > > +
> > > > +       while (duplicates_iterator) {
> > > > +               app = duplicates_iterator;
> > > > +               duplicates_iterator = duplicates_iterator->next;
> > > > +               free(app);
> > > > +       }
> > > > +
> > > > +       *duplicates = NULL;
> > > > +}
> > > > diff --git a/scripts/kas_alias/duplicates_list.h
> > > > b/scripts/kas_alias/duplicates_list.h new file mode 100644
> > > > index 000000000000..76aa73e584bc
> > > > --- /dev/null
> > > > +++ b/scripts/kas_alias/duplicates_list.h
> > > > @@ -0,0 +1,15 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > > +#ifndef DUPLICATES_LIST_H
> > > > +#define DUPLICATES_LIST_H
> > > > +
> > > > +#include "item_list.h"
> > > > +
> > > > +struct duplicate_item {
> > > > +       struct item *original_item;
> > > > +       struct duplicate_item *next;
> > > > +};
> > > > +
> > > > +struct duplicate_item *find_duplicates(struct item *list);
> > > > +void free_duplicates(struct duplicate_item **duplicates);
> > > > +
> > > > +#endif
> > > > diff --git a/scripts/kas_alias/item_list.c b/scripts/kas_alias/item_list.c
> > > > new file mode 100644
> > > > index 000000000000..48f2e525592a
> > > > --- /dev/null
> > > > +++ b/scripts/kas_alias/item_list.c
> > > > @@ -0,0 +1,230 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > > +#include <stdio.h>
> > > > +#include <stdlib.h>
> > > > +#include <stdint.h>
> > > > +#include <string.h>
> > > > +#include <stdbool.h>
> > > > +#include <assert.h>
> > > > +#include "item_list.h"
> > > > +
> > > > +#define CHECK_ORDER_BY_ADDRESS(sort_by, current, temp, op) \
> > > > +       ((sort_by) == BY_ADDRESS && (current)->addr op (temp)->addr)
> > > > +#define CHECK_ORDER_BY_NAME(sort_by, current, temp, op) \
> > > > +       ((sort_by) == BY_NAME && strcmp((current)->symb_name,
> > > > (temp)->symb_name) op 0) +
> > > > +struct item *list_index[96] = {0};
> > > > +
> > > > +void build_index(struct item *list)
> > > > +{
> > > > +       char current_first_letter = ' ';
> > > > +       struct item *current = list;
> > > > +
> > > > +       while (current) {
> > > > +               if (current->symb_name[0] != current_first_letter) {
> > > > +                       current_first_letter = current->symb_name[0];
> > > > +                       list_index[current_first_letter - 32] = current;
> > > > +               }
> > > > +               current = current->next;
> > > > +       }
> > > > +}
> > > > +
> > > > +struct item *add_item(struct item **list, const char *name, char stype,
> > > > uint64_t addr) +{
> > > > +       struct item *new_item;
> > > > +       struct item *current;
> > > > +
> > > > +       new_item = malloc(sizeof(struct item));
> > > > +       if (!new_item)
> > > > +               return NULL;
> > > > +
> > > > +       strncpy(new_item->symb_name, name, MAX_NAME_SIZE);
> > > > +       new_item->symb_name[MAX_NAME_SIZE - 1] = '\0';
> > > > +       new_item->addr = addr;
> > > > +       new_item->stype = stype;
> > > > +       new_item->next = NULL;
> > > > +
> > > > +       if (!(*list)) {
> > > > +               *list = new_item;
> > > > +       } else {
> > > > +               current = *list;
> > > > +               while (current->next)
> > > > +                       current = current->next;
> > > > +
> > > > +               current->next = new_item;
> > > > +       }
> > > > +       return new_item;
> > > > +}
> > > > +
> > > > +void sort_list(struct item **list, int sort_by)
> > > > +{
> > > > +       struct item *current = *list;
> > > > +       struct item *sorted = NULL;
> > > > +       struct item *next_item;
> > > > +       struct item *temp;
> > > > +
> > > > +       if (!(*list) || !((*list)->next))
> > > > +               return;
> > > > +
> > > > +       while (current) {
> > > > +               next_item = current->next;
> > > > +               if (!sorted ||
> > > > +                   (CHECK_ORDER_BY_ADDRESS(sort_by, current, sorted, <)
> > > > ||
> > > > +                   CHECK_ORDER_BY_NAME(sort_by, current, sorted, >=))) {
> > > > +                       current->next = sorted;
> > > > +                       sorted = current;
> > > > +               } else {
> > > > +                       temp = sorted;
> > > > +                       while (temp->next &&
> > > > +                              (CHECK_ORDER_BY_ADDRESS(sort_by, current,
> > > > temp->next, >=) || +
> > > > CHECK_ORDER_BY_NAME(sort_by, current, temp->next, >=))) +
> > > >               temp = temp->next;
> > > > +
> > > > +                       current->next = temp->next;
> > > > +                       temp->next = current;
> > > > +               }
> > > > +               current = next_item;
> > > > +       }
> > > > +
> > > > +       *list = sorted;
> > > > +}
> > > > +
> > > > +struct item *merge(struct item *left, struct item *right, int sort_by)
> > > > +{
> > > > +       struct item *current = NULL;
> > > > +       struct item *result = NULL;
> > > > +
> > > > +       if (!left)
> > > > +               return right;
> > > > +       if (!right)
> > > > +               return left;
> > > > +
> > > > +       if (sort_by == BY_NAME) {
> > > > +               if (strcmp(left->symb_name, right->symb_name) <= 0) {
> > > > +                       result = left;
> > > > +                       left = left->next;
> > > > +               } else {
> > > > +                       result = right;
> > > > +                       right = right->next;
> > > > +               }
> > > > +       } else {
> > > > +               if (sort_by == BY_ADDRESS) {
> > > > +                       if (left->addr <= right->addr) {
> > > > +                               result = left;
> > > > +                               left = left->next;
> > > > +                       } else {
> > > > +                               result = right;
> > > > +                               right = right->next;
> > > > +                       }
> > > > +               }
> > > > +       }
> > > > +
> > > > +       current = result;
> > > > +
> > > > +       while (left && right) {
> > > > +               if (sort_by == BY_NAME) {
> > > > +                       if (strcmp(left->symb_name, right->symb_name) <=
> > > > 0) { +                               current->next = left;
> > > > +                               left = left->next;
> > > > +                       } else {
> > > > +                               current->next = right;
> > > > +                               right = right->next;
> > > > +                       }
> > > > +               } else {
> > > > +                       if (sort_by == BY_ADDRESS) {
> > > > +                               if (left->addr <= right->addr) {
> > > > +                                       current->next = left;
> > > > +                                       left = left->next;
> > > > +                               } else {
> > > > +                                       current->next = right;
> > > > +                                       right = right->next;
> > > > +                               }
> > > > +                       }
> > > > +               }
> > > > +
> > > > +               current = current->next;
> > > > +       }
> > > > +
> > > > +       if (left) {
> > > > +               current->next = left;
> > > > +       } else {
> > > > +               if (right)
> > > > +                       current->next = right;
> > > > +       }
> > > > +
> > > > +       return result;
> > > > +}
> > > > +
> > > > +struct item *merge_sort(struct item *head, int sort_by)
> > > > +{
> > > > +       struct item *right;
> > > > +       struct item *slow;
> > > > +       struct item *fast;
> > > > +       struct item *left;
> > > > +
> > > > +       if (!head || !head->next)
> > > > +               return head;
> > > > +
> > > > +       slow = head;
> > > > +       fast = head->next;
> > > > +
> > > > +       while (fast && fast->next) {
> > > > +               slow = slow->next;
> > > > +               fast = fast->next->next;
> > > > +       }
> > > > +
> > > > +       left = head;
> > > > +       right = slow->next;
> > > > +       slow->next = NULL;
> > > > +
> > > > +       left = merge_sort(left, sort_by);
> > > > +       right = merge_sort(right, sort_by);
> > > > +
> > > > +       return merge(left, right, sort_by);
> > > > +}
> > > > +
> > > > +void sort_list_m(struct item **head, int sort_by)
> > > > +{
> > > > +       if (!(*head) || !((*head)->next))
> > > > +               return;
> > > > +
> > > > +       *head = merge_sort(*head, sort_by);
> > > > +}
> > > > +
> > > > +int insert_after(struct item *list, const uint64_t search_addr,
> > > > +                const char *name, uint64_t addr, char stype)
> > > > +{
> > > > +       struct item *new_item;
> > > > +       struct item *current;
> > > > +       int ret = 0;
> > > > +
> > > > +       current = (list_index[name[0] - 32]) ? list_index[name[0] - 32] :
> > > > list; +       while (current) {
> > > > +               if (current->addr == search_addr) {
> > > > +                       new_item = malloc(sizeof(struct item));
> > > > +                       if (!new_item)
> > > > +                               return ret;
> > > > +                       strncpy(new_item->symb_name, name, MAX_NAME_SIZE);
> > > > +                       new_item->symb_name[MAX_NAME_SIZE - 1] = '\0';
> > > > +                       new_item->addr = addr;
> > > > +                       new_item->stype = stype;
> > > > +                       new_item->next = current->next;
> > > > +                       current->next = new_item;
> > > > +                       ret = 1;
> > > > +                       break;
> > > > +               }
> > > > +               current = current->next;
> > > > +       }
> > > > +       return ret;
> > > > +}
> > > > +
> > > > +void free_items(struct item **head)
> > > > +{
> > > > +       struct item *app, *item_iterator = *head;
> > > > +
> > > > +       while (item_iterator) {
> > > > +               app = item_iterator;
> > > > +               item_iterator = item_iterator->next;
> > > > +               free(app);
> > > > +       }
> > > > +       *head = NULL;
> > > > +}
> > > > diff --git a/scripts/kas_alias/item_list.h b/scripts/kas_alias/item_list.h
> > > > new file mode 100644
> > > > index 000000000000..b4891cb088ee
> > > > --- /dev/null
> > > > +++ b/scripts/kas_alias/item_list.h
> > > > @@ -0,0 +1,26 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > > +#ifndef ITEM_LIST_H
> > > > +#define ITEM_LIST_H
> > > > +#include <stdint.h>
> > > > +
> > > > +#define MAX_NAME_SIZE 256
> > > > +#define BY_ADDRESS 1
> > > > +#define BY_NAME 2
> > > > +
> > > > +struct item {
> > > > +       char            symb_name[MAX_NAME_SIZE];
> > > > +       uint64_t        addr;
> > > > +       char            stype;
> > > > +       struct item     *next;
> > > > +};
> > > > +
> > > > +void build_index(struct item *list);
> > > > +struct item *add_item(struct item **list, const char *name, char stype,
> > > > uint64_t addr); +void sort_list(struct item **list, int sort_by);
> > > > +struct item *merge(struct item *left, struct item *right, int sort_by);
> > > > +struct item *merge_sort(struct item *head, int sort_by);
> > > > +void sort_list_m(struct item **head, int sort_by);
> > > > +int insert_after(struct item *list, const uint64_t search_addr,
> > > > +                const char *name, uint64_t addr, char stype);
> > > > +void free_items(struct item **head);
> > > > +#endif
> > > > diff --git a/scripts/kas_alias/kas_alias.c b/scripts/kas_alias/kas_alias.c
> > > > new file mode 100644
> > > > index 000000000000..532aeb39f851
> > > > --- /dev/null
> > > > +++ b/scripts/kas_alias/kas_alias.c
> > > > @@ -0,0 +1,217 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > > +#include <stdio.h>
> > > > +#include <stdlib.h>
> > > > +#include <stdint.h>
> > > > +#include <unistd.h>
> > > > +#include <string.h>
> > > > +#include <stdbool.h>
> > > > +#include <stdarg.h>
> > > > +#include <regex.h>
> > > > +
> > > > +#include "item_list.h"
> > > > +#include "duplicates_list.h"
> > > > +#include "a2l.h"
> > > > +
> > > > +#define SYMB_IS_TEXT(s) ((((s)->stype) == 't') ||  (((s)->stype) == 'T'))
> > > > +#define SYMB_IS_DATA(s) ((((s)->stype) == 'b') ||  (((s)->stype) == 'B')
> > > > || \ +                        (((s)->stype) == 'd') ||  (((s)->stype) ==
> > > > 'D') || \ +                        (((s)->stype) == 'r') ||
> > > > (((s)->stype) == 'R')) +#ifdef CONFIG_KALLSYMS_ALIAS_DATA
> > > > +#define SYMB_NEEDS_ALIAS(s) (SYMB_IS_TEXT(s) || SYMB_IS_DATA(s))
> > > > +#else
> > > > +#define SYMB_NEEDS_ALIAS(s) SYMB_IS_TEXT(s)
> > > > +#endif
> > > > +#define FNOMATCH 0
> > > > +#define FMATCH 1
> > > > +#define EREGEX 2
> > > > +
> > > > +const char *ignore_regex[] = {
> > > > +       "^__cfi_.*$",                           // __cfi_ preamble
> > > > +#ifndef CONFIG_KALLSYMS_ALIAS_DATA_ALL
> > > > +       "^_*TRACE_SYSTEM.*$",
> > > > +       "^__already_done\\.[0-9]+$",            // Call a function once
> > > > data +       "^___tp_str\\.[0-9]+$",
> > > > +       "^___done\\.[0-9]+$",
> > > > +       "^__print_once\\.[0-9]+$",
> > > > +       "^_rs\\.[0-9]+$",
> > > > +       "^__compound_literal\\.[0-9]+$",
> > > > +       "^___once_key\\.[0-9]+$",
> > > > +       "^__func__\\.[0-9]+$",
> > > > +       "^__msg\\.[0-9]+$",
> > > > +       "^CSWTCH\\.[0-9]+$",
> > > > +       "^__flags\\.[0-9]+$",
> > > > +       "^__wkey.*$",
> > > > +       "^__mkey.*$",
> > > > +       "^__key.*$",
> > > > +#endif
> > > > +       "^__pfx_.*$"                            // NOP-padding
> > > > +};
> > > > +
> > > > +int suffix_serial;
> > > > +
> > > > +static inline void verbose_msg(bool verbose, const char *fmt, ...)
> > > > +{
> > > > +       va_list args;
> > > > +
> > > > +       va_start(args, fmt);
> > > > +       if (verbose)
> > > > +               printf(fmt, args);
> > > > +
> > > > +       va_end(args);
> > > > +}
> > > > +
> > > > +static void create_suffix(const char *name, char *output_suffix)
> > > > +{
> > > > +       sprintf(output_suffix, "%s__alias__%d", name, suffix_serial++);
> > > > +}
> > > > +
> > > > +static void create_file_suffix(const char *name, uint64_t address, char
> > > > *output_suffix, char *cwd) +{
> > > > +       const char *f_path;
> > > > +       char *buf;
> > > > +       int i = 0;
> > > > +
> > > > +       buf = addr2line_get_lines(address);
> > > > +       f_path = remove_subdir(cwd, buf);
> > > > +       if (f_path) {
> > > > +               sprintf(output_suffix, "%s@%s", name, f_path);
> > > > +               while (*(output_suffix + i) != '\0') {
> > > > +                       switch (*(output_suffix + i)) {
> > > > +                       case '/':
> > > > +                       case ':':
> > > > +                       case '.':
> > > > +                               *(output_suffix + i) = '_';
> > > > +                               break;
> > > > +                       default:
> > > > +                       }
> > > > +               i++;
> > > > +               }
> > > > +       } else {
> > > > +               create_suffix(name, output_suffix);
> > > > +       }
> > > > +}
> > > > +
> > > > +static int filter_symbols(char *symbol, const char **ignore_list, int
> > > > regex_no) +{
> > > > +       regex_t regex;
> > > > +       int res, i;
> > > > +
> > > > +       for (i = 0; i < regex_no; i++) {
> > > > +               res = regcomp(&regex, ignore_list[i], REG_EXTENDED);
> > > > +               if (res)
> > > > +                       return -EREGEX;
> > > > +
> > > > +               res = regexec(&regex, symbol, 0, NULL, 0);
> > > > +               regfree(&regex);
> > > > +               switch (res) {
> > > > +               case 0:
> > > > +                       return FMATCH;
> > > > +               case REG_NOMATCH:
> > > > +                       break;
> > > > +               default:
> > > > +                       return -EREGEX;
> > > > +               }
> > > > +       }
> > > > +
> > > > +       return FNOMATCH;
> > > > +}
> > > > +
> > > > +int main(int argc, char *argv[])
> > > > +{
> > > > +       char t, sym_name[MAX_NAME_SIZE], new_name[MAX_NAME_SIZE + 15];
> > > > +       struct duplicate_item  *duplicate_iterator;
> > > > +       struct duplicate_item *duplicate;
> > > > +       struct item *head = {NULL};
> > > > +       bool need_2_process = true;
> > > > +       struct item *last = {NULL};
> > > > +       struct item  *current;
> > > > +       int verbose_mode = 0;
> > > > +       uint64_t address;
> > > > +       FILE *fp;
> > > > +       int res;
> > > > +
> > > > +       if (argc < 2 || argc > 3) {
> > > > +               printf("Usage: %s <nmfile> [-verbose]\n", argv[0]);
> > > > +               return 1;
> > > > +       }
> > > > +
> > > > +       if (argc == 3 && strcmp(argv[2], "-verbose") == 0)
> > > > +               verbose_mode = 1;
> > > > +
> > > > +       verbose_msg(verbose_mode, "Scanning nm data(%s)\n", argv[1]);
> > > > +
> > > > +       fp = fopen(argv[1], "r");
> > > > +       if (!fp) {
> > > > +               printf("Can't open input file.\n");
> > > > +               return 1;
> > > > +       }
> > > > +
> > > > +       if (!addr2line_init(get_addr2line(A2L_DEFAULT),
> > > > get_vmlinux(argv[1]))) +               return 1;
> > > > +
> > > > +       while (fscanf(fp, "%lx %c %99s\n", &address, &t, sym_name) == 3) {
> > > > +               if (strstr(sym_name, "@_")) {
> > > > +                       if (verbose_mode && need_2_process)
> > > > +                               printf("Already processed\n");
> > > > +                       need_2_process = false;
> > > > +                       }
> > > > +               last = add_item(&last, sym_name, t, address);
> > > > +               if (!last) {
> > > > +                       printf("Error in allocate memory\n");
> > > > +                       free_items(&head);
> > > > +                       return 1;
> > > > +               }
> > > > +
> > > > +               if (!head)
> > > > +                       head = last;
> > > > +       }
> > > > +
> > > > +       fclose(fp);
> > > > +
> > > > +       if (need_2_process) {
> > > > +               verbose_msg(verbose_mode, "Sorting nm data\n");
> > > > +               sort_list_m(&head, BY_NAME);
> > > > +               verbose_msg(verbose_mode, "Scanning nm data for
> > > > duplicates\n"); +               duplicate = find_duplicates(head);
> > > > +               if (!duplicate) {
> > > > +                       printf("Error in duplicates list\n");
> > > > +                       return 1;
> > > > +               }
> > > > +
> > > > +               verbose_msg(verbose_mode, "Applying suffixes\n");
> > > > +               build_index(head);
> > > > +               duplicate_iterator = duplicate;
> > > > +               while (duplicate_iterator) {
> > > > +                       res =
> > > > filter_symbols(duplicate_iterator->original_item->symb_name, +
> > > >                                 ignore_regex, sizeof(ignore_regex) / +
> > > >                                         sizeof(ignore_regex[0])); +
> > > >                 if (res != FMATCH &&
> > > > +
> > > > SYMB_NEEDS_ALIAS(duplicate_iterator->original_item)) { +
> > > >              if (res < 0)
> > > > +                                       return 1;
> > > > +
> > > > +
> > > > create_file_suffix(duplicate_iterator->original_item->symb_name, +
> > > >
> > > > duplicate_iterator->original_item->addr, +
> > > >                   new_name, vmlinux_path); +
> > > >  if (!insert_after(head, duplicate_iterator->original_item->addr, +
> > > >                                           new_name,
> > > > duplicate_iterator->original_item->addr, +
> > > >                  duplicate_iterator->original_item->stype)) +
> > > >                           return 1;
> > > > +                       }
> > > > +
> > > > +                       duplicate_iterator = duplicate_iterator->next;
> > > > +               }
> > > > +
> > > > +               sort_list_m(&head, BY_ADDRESS);
> > > > +       }
> > > > +       current = head;
> > > > +       while (current) {
> > > > +               printf("%08lx %c %s\n", current->addr, current->stype,
> > > > current->symb_name); +               current = current->next;
> > > > +       }
> > > > +
> > > > +       free_items(&head);
> > > > +       free_duplicates(&duplicate);
> > > > +       addr2line_cleanup();
> > > > +       return 0;
> > > > +}
> > > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > > > index a432b171be82..cacf60b597ce 100755
> > > > --- a/scripts/link-vmlinux.sh
> > > > +++ b/scripts/link-vmlinux.sh
> > > > @@ -89,8 +89,9 @@ vmlinux_link()
> > > >
> > > >         ldflags="${ldflags} ${wl}--script=${objtree}/${KBUILD_LDS}"
> > > >
> > > > -       # The kallsyms linking does not need debug symbols included.
> > > > -       if [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
> > > > +       # The kallsyms linking does not need debug symbols included,
> > > > unless the KALLSYMS_ALIAS. +       if [ ! is_enabled
> > > > CONFIG_KALLSYMS_ALIAS ] && \
> > > > +           [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
> > > >
> > > >                 ldflags="${ldflags} ${wl}--strip-debug"
> > > >
> > > >         fi
> > > >
> > > > @@ -161,7 +162,11 @@ kallsyms()
> > > >
> > > >         fi
> > > >
> > > >         info KSYMS ${2}
> > > >
> > > > -       scripts/kallsyms ${kallsymopt} ${1} > ${2}
> > > > +       if is_enabled CONFIG_KALLSYMS_ALIAS; then
> > > > +               ALIAS=".alias"
> > > > +               scripts/kas_alias/kas_alias ${1} >${1}${ALIAS}
> > > > +               fi
> > > > +       scripts/kallsyms ${kallsymopt} ${1}${ALIAS} > ${2}
> > > >
> > > >  }
> > > >
> > > >  # Perform one step in kallsyms generation, including temporary linking of
> > > >
> > > > --
> > > > 2.34.1
> >
> > Best regards.
> >
> >
> BR
> Alessandro


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

^ permalink raw reply

* Re: [PATCH v3 1/1] tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols
From: Masami Hiramatsu @ 2023-08-25 12:16 UTC (permalink / raw)
  To: Francis Laniel
  Cc: linux-kernel, Masami Hiramatsu, linux-trace-kernel,
	Steven Rostedt
In-Reply-To: <20230824160859.66113-2-flaniel@linux.microsoft.com>

On Thu, 24 Aug 2023 18:08:59 +0200
Francis Laniel <flaniel@linux.microsoft.com> wrote:

> 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.
> 
> Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
> Link: https://lore.kernel.org/lkml/20230819101105.b0c104ae4494a7d1f2eea742@kernel.org/
> ---

Ah, this should be fine, but selftest (tools/testing/selftests/ftrace) fails.

 # tail 60-kprobe_module.tc-log.vsOHnF 
...
+ :
+ : 'Add an event on a module function without specifying event name'
+ :
+ echo 'p trace_printk:trace_printk_irq_work'
sh: write error: No such file or directory

Ah, the function on non-exist module should be checked too.

# tail 63-kprobe_syntax_errors.tc-log.mMLwIQ 
+ + printfwc '%s' -c
 'p '
+ pos=2
+ printf+  '%s'tr 'p ^non_exist_func'
 -d ^
+ command='p non_exist_func'
+ echo 'Test command: p non_exist_func'
Test command: p non_exist_func
+ echo
+ grep 'trace_kprobe: error:' -A 3 error_log

Also, this doesn't leave a syntax error message.

So, the below changes are needed.

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 8ab46a2a446d..1e57bc896952 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -855,7 +855,7 @@ static int __trace_kprobe_create(int argc, const char *argv[])
 		}
 	}
 
-	if (symbol) {
+	if (symbol && !strchr(symbol, ':')) {
 		unsigned int count;
 
 		count = number_of_same_symbols(symbol);
@@ -864,6 +864,7 @@ static int __trace_kprobe_create(int argc, const char *argv[])
 			 * Users should use ADDR to remove the ambiguity of
 			 * using KSYM only.
 			 */
+			trace_probe_log_err(0, NON_UNIQ_SYMBOL);
 			ret = -EADDRNOTAVAIL;
 
 			goto error;
@@ -872,6 +873,7 @@ static int __trace_kprobe_create(int argc, const char *argv[])
 			 * We can return ENOENT earlier than when register the
 			 * kprobe.
 			 */
+			trace_probe_log_err(0, BAD_PROBE_ADDR);
 			ret = -ENOENT;
 
 			goto error;
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 7f929482e8d4..a4f478448eef 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -450,6 +450,7 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call,
 	C(BAD_MAXACT,		"Invalid maxactive number"),		\
 	C(MAXACT_TOO_BIG,	"Maxactive is too big"),		\
 	C(BAD_PROBE_ADDR,	"Invalid probed address or symbol"),	\
+	C(NON_UNIQ_SYMBOL,	"The symbol is not unique"),		\
 	C(BAD_RETPROBE,		"Retprobe address must be an function entry"), \
 	C(NO_TRACEPOINT,	"Tracepoint is not found"),		\
 	C(BAD_ADDR_SUFFIX,	"Invalid probed address suffix"), \

Thank you,

>  kernel/trace/trace_kprobe.c | 61 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 23dba01831f7..2f393739e8cf 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -705,6 +705,25 @@ static struct notifier_block trace_kprobe_module_nb = {
>  	.priority = 1	/* Invoked after kprobe module callback */
>  };
>  
> +static int count_symbols(void *data, unsigned long unused)
> +{
> +	unsigned int *count = data;
> +
> +	(*count)++;
> +
> +	return 0;
> +}
> +
> +static unsigned int number_of_same_symbols(char *func_name)
> +{
> +	unsigned int count;
> +
> +	count = 0;
> +	kallsyms_on_each_match_symbol(count_symbols, func_name, &count);
> +
> +	return count;
> +}
> +
>  static int __trace_kprobe_create(int argc, const char *argv[])
>  {
>  	/*
> @@ -836,6 +855,29 @@ static int __trace_kprobe_create(int argc, const char *argv[])
>  		}
>  	}
>  
> +	if (symbol) {
> +		unsigned int count;
> +
> +		count = number_of_same_symbols(symbol);
> +		if (count > 1) {
> +			/*
> +			 * Users should use ADDR to remove the ambiguity of
> +			 * using KSYM only.
> +			 */

			

> +			ret = -EADDRNOTAVAIL;
> +
> +			goto error;
> +		} else if (count == 0) {
> +			/*
> +			 * We can return ENOENT earlier than when register the
> +			 * kprobe.
> +			 */
> +			ret = -ENOENT;
> +
> +			goto error;
> +		}
> +	}
> +
>  	trace_probe_log_set_index(0);
>  	if (event) {
>  		ret = traceprobe_parse_event_name(&event, &group, gbuf,
> @@ -1699,6 +1741,7 @@ static int unregister_kprobe_event(struct trace_kprobe *tk)
>  }
>  
>  #ifdef CONFIG_PERF_EVENTS
> +
>  /* create a trace_kprobe, but don't add it to global lists */
>  struct trace_event_call *
>  create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
> @@ -1709,6 +1752,24 @@ create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
>  	int ret;
>  	char *event;
>  
> +	if (func) {
> +		unsigned int count;
> +
> +		count = number_of_same_symbols(func);
> +		if (count > 1)
> +			/*
> +			 * Users should use addr to remove the ambiguity of
> +			 * using func only.
> +			 */
> +			return ERR_PTR(-EADDRNOTAVAIL);
> +		else if (count == 0)
> +			/*
> +			 * We can return ENOENT earlier than when register the
> +			 * kprobe.
> +			 */
> +			return ERR_PTR(-ENOENT);
> +	}
> +
>  	/*
>  	 * local trace_kprobes are not added to dyn_event, so they are never
>  	 * searched in find_trace_kprobe(). Therefore, there is no concern of
> -- 
> 2.34.1
> 


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

^ permalink raw reply related

* [PATCH] trace/hwlat: remove extra space at the end of hwlat_detector/mode
From: Mikhail Kobuk @ 2023-08-25 10:34 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Mikhail Kobuk, Daniel Bristot de Oliveira, Masami Hiramatsu,
	linux-kernel, linux-trace-kernel, lvc-project, Alexey Khoroshilov

Space is printed after each mode value including the last one:
$ echo \"$(sudo cat /sys/kernel/tracing/hwlat_detector/mode)\"
"none [round-robin] per-cpu "

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 8fa826b7344d ("trace/hwlat: Implement the mode config option")
Signed-off-by: Mikhail Kobuk <m.kobuk@ispras.ru>
Reviewed-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
 kernel/trace/trace_hwlat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
index 2f37a6e68aa9..b791524a6536 100644
--- a/kernel/trace/trace_hwlat.c
+++ b/kernel/trace/trace_hwlat.c
@@ -635,7 +635,7 @@ static int s_mode_show(struct seq_file *s, void *v)
 	else
 		seq_printf(s, "%s", thread_mode_str[mode]);
 
-	if (mode != MODE_MAX)
+	if (mode < MODE_MAX - 1) /* if mode is any but last */
 		seq_puts(s, " ");
 
 	return 0;
-- 
2.42.0


^ permalink raw reply related

* Re: [PATCH v2] scripts/link-vmlinux.sh: Add alias to duplicate symbols for kallsyms
From: Alessandro Carminati @ 2023-08-25 10:15 UTC (permalink / raw)
  To: Francis Laniel
  Cc: Steven Rostedt, Alexander Lobakin, Nick Alcock, Masahiro Yamada,
	Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Masami Hiramatsu, Daniel Bristot de Oliveira, Viktor Malik,
	Kris Van Hees, Luis Chamberlain, eugene.loh, Josh Poimboeuf,
	linux-kernel, linux-kbuild, live-patching, linux-trace-kernel
In-Reply-To: <1862253.tdWV9SEqCh@pwmachine>

Hello Francis,
Thanks for your input!

Il giorno gio 24 ago 2023 alle ore 17:35 Francis Laniel
<flaniel@linux.microsoft.com> ha scritto:
>
> Hi.
>
> Le vendredi 21 juillet 2023, 14:40:54 CEST Alessandro Carminati a écrit :
> > Hello,
> >
> > I apologize for being noisy today.
> >
> > In an effort to be collaborative, I would like to share my thoughts on why I
> > see duplicate symbols in fs/binfmt_elf.c.
> >
> > Il giorno ven 21 lug 2023 alle ore 11:22 Alessandro Carminati
> >
> > <alessandro.carminati@gmail.com> ha scritto:
> > > Hello Steven, Alexander, and Nick,
> > >
> > > Since now I realize that a lot of people are working on my very same
> > > argument, I'm writing you this mail just with the intent of not leaving
> > > my statements floating in the air, and help the cause by sharing my
> > > experience, as little it is, to solve this problem.
> > >
> > > The addr2line strategy, I tried to implement, allows me to achieve the
> > > level of accuracy that Luis mentioned in his comment about live-patching
> > > suggestions. The addr2line-based approach is feasible, however, to make
> > > this approach operable, I had to ensure that vmlinux contains the
> > > necessary debug information.
> > > Let me clarify.
> > > During the kernel build, I do need the vmlinux to contain debug
> > > information. The approach I pursued parses the nm output and, when it
> > > detects a duplicate symbol, it needs to query vmlinux using addr2line to
> > > resolve the address where the symbol is reported to be.
> > > The whole process takes place during the building phase.
> > > I've completed the work to integrate the new alias within the compilation
> > > phase, and I wanted to share with you how it looks now.
> > >
> > > ```
> > > ~ # uname -a
> > > Linux (none) 6.4.0 #1 SMP PREEMPT Thu Jul 20 10:05:36 UTC 2023 aarch64
> > > GNU/Linux ~ # cat /proc/kallsyms | grep gic_mask_irq
> > > ffffacf2eb64dae4 t gic_mask_irq
> > > ffffacf2eb64dae4 t gic_mask_irq@_drivers_irqchip_irq-gic_c_167
> > > ffffacf2eb650960 t gic_mask_irq
> > > ffffacf2eb650960 t gic_mask_irq@_drivers_irqchip_irq-gic-v3_c_404
> > > ~ #
> > > ```
> > >
> > > Despite this appearing to have a sufficient level of accuracy, when I
> > > tested it, I realized something I really did not expect. Multiple
> > > instances of the same function exist, and when I say "same function," I
> > > don't just mean the name, but also the function's body. I apologize for
> > > stating something that may sound obvious to you, but I found this really
> > > unexpected.
> > >
> > > ```
> > > ~ # uname -a
> > > Linux (none) 6.4.0 #1 SMP PREEMPT Thu Jul 20 10:05:36 UTC 2023 aarch64
> > > GNU/Linux ~ # cat /proc/kallsyms | grep -E " [ttT] " | cut -d" "
> > > -f3|sort| uniq -d| grep @_
> > > BIT_initDStream@_lib_zstd_common_bitstream_h_259
> > > __dev_hold@_include_linux_netdevice_h_4059
> > > __dev_put@_include_linux_netdevice_h_4048
> > > __flush_tlb_range@_arch_arm64_include_asm_tlbflush_h_291
> > > __flush_tlb_range_constprop_0@_arch_arm64_include_asm_tlbflush_h_291
> > > __kern_my_cpu_offset@_arch_arm64_include_asm_percpu_h_40
> > > __kvm_nvhe_$x@_arch_arm64_kvm_hyp_include_nvhe_memory_h_22
> > > __kvm_nvhe___kvm_skip_instr@_arch_arm64_kvm_hyp_include_hyp_adjust_pc_h_34
> > > __kvm_nvhe_hyp_page_count@_arch_arm64_kvm_hyp_include_nvhe_memory_h_47
> > > __kvm_nvhe_hyp_phys_to_virt@_arch_arm64_kvm_hyp_include_nvhe_memory_h_22
> > > __kvm_nvhe_hyp_virt_to_phys@_arch_arm64_kvm_hyp_include_nvhe_memory_h_27
> > > __kvm_skip_instr@_arch_arm64_kvm_hyp_include_hyp_adjust_pc_h_34
> > > __nodes_weight_constprop_0@_include_linux_nodemask_h_239
> > > __pi_$x@_scripts_dtc_libfdt_libfdt_h_145
> > > __pi_fdt32_ld@_scripts_dtc_libfdt_libfdt_h_145
> > > __preempt_count_dec_and_test@_arch_arm64_include_asm_current_h_19
> > > acpi_dev_filter_resource_type_cb@_include_linux_acpi_h_520
> > > add_quirk@_drivers_mmc_core_card_h_158
> > > bpf_enable_instrumentation@_include_linux_bpf_h_1937
> > > btf_id_cmp_func@_include_linux_btf_h_469
> > > copy_from_sockptr_offset_constprop_0@_include_linux_sockptr_h_44
> > > cpucap_multi_entry_cap_matches@_arch_arm64_include_asm_cpufeature_h_395
> > > cpufreq_register_em_with_opp@_include_linux_cpufreq_h_1228
> > > cpumask_weight@_include_linux_cpumask_h_691
> > > cpumask_weight_constprop_0@_include_linux_cpumask_h_690
> > > css_put@_include_linux_cgroup_refcnt_h_77
> > > dma_cookie_status@_drivers_dma_dmaengine_h_74
> > > dst_discard@_include_net_dst_h_392
> > > dst_output@_include_net_dst_h_457
> > > elf_core_dump@_fs_binfmt_elf_c_2027
> > > fixup_use_fwh_lock@_drivers_mtd_chips_fwh_lock_h_102
> > > flush_tlb_mm@_arch_arm64_include_asm_tlbflush_h_250
> > > fwh_lock_varsize@_drivers_mtd_chips_fwh_lock_h_81
> > > fwh_unlock_varsize@_drivers_mtd_chips_fwh_lock_h_92
> > > fwh_xxlock_oneblock@_drivers_mtd_chips_fwh_lock_h_31
> > > get_net_ns@_include_net_net_namespace_h_231
> > > inline_map_read_isra_0@_include_linux_mtd_map_h_393
> > > inline_map_write_part_0@_include_linux_mtd_map_h_426
> > > io_run_task_work@_io_uring_io_uring_h_282
> > > ipv6_portaddr_hash_isra_0@_include_net_ipv6_h_732
> > > irq_find_host@_include_linux_irqdomain_h_322
> > > jhash@_include_linux_jhash_h_76
> > > jhash_constprop_0@_include_linux_jhash_h_76
> > > ktime_get_boottime@_include_linux_timekeeping_h_94
> > > ktime_get_real@_include_linux_timekeeping_h_78
> > > load_elf_binary@_fs_binfmt_elf_c_824
> > > load_elf_phdrs@_fs_binfmt_elf_c_468
> > > may_use_simd@_arch_arm64_include_asm_alternative-macros_h_232
> > > netif_tx_disable@_include_linux_netdevice_h_4486
> > > of_parse_phandle@_include_linux_of_h_946
> > > of_parse_phandle_constprop_0@_include_linux_of_h_943
> > > padzero@_fs_binfmt_elf_c_142
> > > percpu_down_read_constprop_0@_include_linux_percpu-rwsem_h_47
> > > percpu_ref_get_many@_include_linux_percpu-refcount_h_199
> > > percpu_ref_get_many_constprop_0@_include_linux_percpu-refcount_h_198
> > > percpu_ref_put_many_constprop_0@_include_linux_percpu-refcount_h_326
> > > percpu_up_read@_include_linux_percpu-rwsem_h_98
> > > percpu_up_read_constprop_0@_include_linux_percpu-rwsem_h_97
> > > pinconf_generic_dt_node_to_map_all@_include_linux_pinctrl_pinconf-generic_
> > > h_223
> > > pinconf_generic_dt_node_to_map_group@_include_linux_pinctrl_pinconf-gener
> > > ic_h_207
> > > pinconf_generic_dt_node_to_map_pin@_include_linux_pinctrl_pinconf-generic
> > > _h_215
> > > platform_device_register_resndata_constprop_0@_include_linux_platform_dev
> > > ice_h_125
> > > ptrauth_keys_install_user@_arch_arm64_include_asm_alternative-macros_h_23
> > > 2 readl@_arch_arm64_include_asm_io_h_75
> > > reqsk_put@_include_net_request_sock_h_133
> > > rht_key_get_hash_constprop_0_isra_0@_include_linux_rhashtable_h_133
> > > rht_key_get_hash_isra_0@_include_linux_rhashtable_h_125
> > > role_show@_include_linux_device_h_763
> > > sdhci_and_cqhci_reset@_drivers_mmc_host_sdhci-cqhci_h_16
> > > set_active_memcg_part_0@_include_linux_sched_mm_h_410
> > > set_brk@_fs_binfmt_elf_c_114
> > > set_is_seen@_arch_arm64_include_asm_current_h_19
> > > set_lookup@_arch_arm64_include_asm_current_h_19
> > > sha1_base_init@_include_crypto_sha1_base_h_22
> > > sha224_base_init@_include_crypto_sha256_base_h_23
> > > sha256_base_init@_include_crypto_sha256_base_h_31
> > > spi_sync_transfer_constprop_0@_include_linux_spi_spi_h_1339
> > > spi_write@_include_linux_spi_spi_h_1363
> > > tlb_flush@_arch_arm64_include_asm_tlb_h_54
> > > trace_xhci_dbg_quirks@_drivers_usb_host_xhci-trace_h_48
> > > udp_lib_close@_include_net_udp_h_197
> > > udp_lib_hash@_include_net_udp_h_189
> > > virtio_net_hdr_to_skb_constprop_0@_include_linux_virtio_net_h_48
> > > writenote@_fs_binfmt_elf_c_1479
> > > zero_user_segments@_include_linux_highmem_h_271
> > > zero_user_segments_constprop_0@_include_linux_highmem_h_268
> > > ```
> > >
> > > The previous evidence demonstrates that adding a tag with a file and line
> > > number does not make a symbol unique.
> > >
> > > Here, I inspect the function body to verify if the functions are indeed
> > > the same.
> > >
> > > ```
> > > Welcome to Buildroot
> > > buildroot login: root
> > > ~ # cat /proc/kallsyms | grep set_brk
> > > ffffa8a9d1cf1d2c t set_brk
> > > ffffa8a9d1cf1d2c t set_brk@_fs_binfmt_elf_c_114
> > > ffffa8a9d1cf4454 t set_brk
> > > ffffa8a9d1cf4454 t set_brk@_fs_binfmt_elf_c_114
> > > ~ # QEMU: Terminated
> > > $ cat System.map | grep set_brk
> > > ffff8000082f1d2c t set_brk
> > > ffff8000082f4454 t set_brk
> > > ~ $ r2 vmlinux
> > > Warning: run r2 with -e bin.cache=true to fix relocations in disassembly
> > >
> > >  -- Reduce the delta where flag resolving by address is used with
> > >  cfg.delta
> > >
> > > [0xffff800008000000]> s 0xffff8000082f1d2c
> > > [0xffff8000082f1d2c]> aa
> > > [x] Analyze all flags starting with sym. and entry0 (aa)
> > > [x] Analyze all functions arguments/locals
> > > [0xffff8000082f1d2c]> pdf
> > >
> > >             ; CALL XREFS from sym.load_elf_binary @ 0xffff8000082f352c(x),
> > >             0xffff8000082f38d4(x), 0xffff8000082f3a68(x)>
> > > ┌ 112: sym.set_brk (int64_t arg1, int64_t arg2, int64_t arg3);
> > > │           ; arg int64_t arg1 @ x0
> > > │           ; arg int64_t arg2 @ x1
> > > │           ; arg int64_t arg3 @ x2
> > > │           ; var int64_t var_10h @ sp+0x10
> > > │           0xffff8000082f1d2c      3f2303d5       paciasp             ;
> > > binfmt_elf.c:114 │           0xffff8000082f1d30      fd7bbea9       stp
> > > x29, x30, [sp, -0x20]! │           0xffff8000082f1d34      00fc3f91
> > > add x0, x0, 0xfff   ; binfmt_elf.c:115 ; arg1 │
> > > 0xffff8000082f1d38      fd030091       mov x29, sp         ;
> > > binfmt_elf.c:114 │           0xffff8000082f1d3c      21fc3f91       add
> > > x1, x1, 0xfff   ; binfmt_elf.c:116 ; arg2 │           0xffff8000082f1d40
> > >     00cc7492       and x0, x0, 0xfffffffffffff000 ; binfmt_elf.c:114 ;
> > > arg1 │           0xffff8000082f1d44      f30b00f9       str x19, [sp,
> > > 0x10] ; binfmt_elf.c:116 │           0xffff8000082f1d48      33cc7492
> > >   and x19, x1, 0xfffffffffffff000 ; arg2 │           0xffff8000082f1d4c
> > >    1f0013eb       cmp x0, x19         ; binfmt_elf.c:117 ; arg1 │
> > > ┌─< 0xffff8000082f1d50      63010054       b.lo 0xffff8000082f1d7c │
> > > ┌──> 0xffff8000082f1d54      014138d5       mrs x1, sp_el0      ;
> > > binfmt_elf.c:128 │      ╎│   0xffff8000082f1d58      22fc41f9       ldr
> > > x2, [x1, 0x3f8] ; current.h:21 ; 0xd9 ; 217 │      ╎│
> > > 0xffff8000082f1d5c      00008052       mov w0, 0           ;
> > > binfmt_elf.c:129 │      ╎│   0xffff8000082f1d60      539400f9       str
> > > x19, [x2, 0x128] ; binfmt_elf.c:128 │      ╎│   0xffff8000082f1d64
> > > 21fc41f9       ldr x1, [x1, 0x3f8] ; current.h:17 ; 0xd9 ; 217 │      ╎│
> > >  0xffff8000082f1d68      339000f9       str x19, [x1, 0x120] ;
> > > binfmt_elf.c:128 │      ╎│   0xffff8000082f1d6c      f30b40f9       ldr
> > > x19, [var_10h]  ; binfmt_elf.c:129 ; 5 │      ╎│   0xffff8000082f1d70
> > >  fd7bc2a8       ldp x29, x30, [sp], 0x20 │      ╎│   0xffff8000082f1d74
> > >    bf2303d5       autiasp
> > > │      ╎│   0xffff8000082f1d78      c0035fd6       ret
> > > │      ╎└─> 0xffff8000082f1d7c      42007e92       and x2, x2, 4       ;
> > > binfmt_elf.c:123 ; arg3 │      ╎    0xffff8000082f1d80      610200cb
> > >  sub x1, x19, x0     ; arg1 │      ╎    0xffff8000082f1d84      cae3fc97
> > >      bl sym.vm_brk_flags │      └──< 0xffff8000082f1d88      60feff34
> > >   cbz w0, 0xffff8000082f1d54 ; binfmt_elf.c:125 │
> > > 0xffff8000082f1d8c      f30b40f9       ldr x19, [var_10h]  ;
> > > binfmt_elf.c:130 ; 5 │           0xffff8000082f1d90      fd7bc2a8
> > > ldp x29, x30, [sp], 0x20 │           0xffff8000082f1d94      bf2303d5
> > >   autiasp
> > > └           0xffff8000082f1d98      c0035fd6       ret
> > > [0xffff8000082f1d2c]> s  0xffff8000082f4454
> > > [0xffff8000082f4454]> pdf
> > >
> > >             ; CALL XREFS from sym.load_elf_binary_1 @
> > >             0xffff8000082f5cc0(x), 0xffff8000082f5e5c(x),
> > >             0xffff8000082f6648(x)>
> > > ┌ 112: sym.set_brk_1 (int64_t arg1, int64_t arg2, int64_t arg3);
> > > │           ; arg int64_t arg1 @ x0
> > > │           ; arg int64_t arg2 @ x1
> > > │           ; arg int64_t arg3 @ x2
> > > │           ; var int64_t var_10h @ sp+0x10
> > > │           0xffff8000082f4454      3f2303d5       paciasp             ;
> > > binfmt_elf.c:114 │           0xffff8000082f4458      fd7bbea9       stp
> > > x29, x30, [sp, -0x20]! │           0xffff8000082f445c      00fc3f91
> > > add x0, x0, 0xfff   ; binfmt_elf.c:115 ; arg1 │
> > > 0xffff8000082f4460      fd030091       mov x29, sp         ;
> > > binfmt_elf.c:114 │           0xffff8000082f4464      21fc3f91       add
> > > x1, x1, 0xfff   ; binfmt_elf.c:116 ; arg2 │           0xffff8000082f4468
> > >     00cc7492       and x0, x0, 0xfffffffffffff000 ; binfmt_elf.c:114 ;
> > > arg1 │           0xffff8000082f446c      f30b00f9       str x19, [sp,
> > > 0x10] ; binfmt_elf.c:116 │           0xffff8000082f4470      33cc7492
> > >   and x19, x1, 0xfffffffffffff000 ; arg2 │           0xffff8000082f4474
> > >    1f0013eb       cmp x0, x19         ; binfmt_elf.c:117 ; arg1 │
> > > ┌─< 0xffff8000082f4478      63010054       b.lo 0xffff8000082f44a4 │
> > > ┌──> 0xffff8000082f447c      014138d5       mrs x1, sp_el0      ;
> > > binfmt_elf.c:128 │      ╎│   0xffff8000082f4480      22fc41f9       ldr
> > > x2, [x1, 0x3f8] ; current.h:21 ; 0xd9 ; 217 │      ╎│
> > > 0xffff8000082f4484      00008052       mov w0, 0           ;
> > > binfmt_elf.c:129 │      ╎│   0xffff8000082f4488      539400f9       str
> > > x19, [x2, 0x128] ; binfmt_elf.c:128 │      ╎│   0xffff8000082f448c
> > > 21fc41f9       ldr x1, [x1, 0x3f8] ; current.h:17 ; 0xd9 ; 217 │      ╎│
> > >  0xffff8000082f4490      339000f9       str x19, [x1, 0x120] ;
> > > binfmt_elf.c:128 │      ╎│   0xffff8000082f4494      f30b40f9       ldr
> > > x19, [var_10h]  ; binfmt_elf.c:129 ; 5 │      ╎│   0xffff8000082f4498
> > >  fd7bc2a8       ldp x29, x30, [sp], 0x20 │      ╎│   0xffff8000082f449c
> > >    bf2303d5       autiasp
> > > │      ╎│   0xffff8000082f44a0      c0035fd6       ret
> > > │      ╎└─> 0xffff8000082f44a4      42007e92       and x2, x2, 4       ;
> > > binfmt_elf.c:123 ; arg3 │      ╎    0xffff8000082f44a8      610200cb
> > >  sub x1, x19, x0     ; arg1 │      ╎    0xffff8000082f44ac      00dafc97
> > >      bl sym.vm_brk_flags │      └──< 0xffff8000082f44b0      60feff34
> > >   cbz w0, 0xffff8000082f447c ; binfmt_elf.c:125 │
> > > 0xffff8000082f44b4      f30b40f9       ldr x19, [var_10h]  ;
> > > binfmt_elf.c:130 ; 5 │           0xffff8000082f44b8      fd7bc2a8
> > > ldp x29, x30, [sp], 0x20 │           0xffff8000082f44bc      bf2303d5
> > >   autiasp
> > > └           0xffff8000082f44c0      c0035fd6       ret
> > > [0xffff8000082f4454]>
> > > ```
> > >
> > > While I can speculate on a reasonable explanation for why I see symbols
> > > coming from header files being duplicated – when a header file is
> > > included in a C file and it produces a function that the compiler does
> > > not want to inline, it is replicated every time the file is included – I
> > > have no convincing explanation as to why the fs/binfmt.c functions exist
> > > in more than one instance.
> > >
> > > ```
> > > elf_core_dump@_fs_binfmt_elf_c_2027
> > > load_elf_binary@_fs_binfmt_elf_c_824
> > > load_elf_phdrs@_fs_binfmt_elf_c_468
> > > padzero@_fs_binfmt_elf_c_142
> > > set_brk@_fs_binfmt_elf_c_114
> > > writenote@_fs_binfmt_elf_c_1479
> > > ```
> >
> > After examining the contents of built-in.a, I have come up with a new
> > interpretation of what I am observing.
> > According to built-in.a, System.map, and vmlinux there are two symbols named
> > set_brk.
> >
> > ```
> > ~ $ cat System.map | grep set_brk
> > ffff8000082f1d2c t set_brk
> > ffff8000082f4454 t set_brk
> > ~ $
> > ~ $ nm -n vmlinux | grep set_brk
> > ffff8000082f1d2c t set_brk
> > ffff8000082f4454 t set_brk
> > ~ $
> > ~ $ nm -n built-in.a| grep set_brk
> > 00000000000000d4 t set_brk
> > 00000000000000d4 t set_brk
> > ~ $
> > ~ $ nm -n built-in.a | grep set_brk -B100| egrep "set_brk|\.o:$"
> > fs/binfmt_elf.o:
> > 00000000000000d4 t set_brk
> > fs/compat_binfmt_elf.o:
> > 00000000000000d4 t set_brk
> > ```
> >
> > These two symbols come from fs/binfmt_elf.o and fs/compat_binfmt_elf.o, and
> > they are just two symbols that happen to share the same name, as is common
> > in the kernel.
> >
> > at the same time, addr2line reports symbols to be generated from the same
> > file.
> >
> > ```
> > ~ $ llvm-addr2line-14 -fe vmlinux ffff8000082f1d2c ffff8000082f4454
> > set_brk
> > /home/alessandro/src/lka64/linux-6.4/fs/binfmt_elf.c:114
> > set_brk
> > /home/alessandro/src/lka64/linux-6.4/fs/binfmt_elf.c:114
> > ~ $
> > ~ $ addr2line -fe vmlinux ffff8000082f1d2c ffff8000082f4454
> > set_brk
> > /home/alessandro/src/lka64/linux-6.4/fs/binfmt_elf.c:114
> > set_brk
> > /home/alessandro/src/lka64/linux-6.4/fs/binfmt_elf.c:114
> > ```
> > looking at the source code:
> > https://elixir.bootlin.com/linux/v6.4/source/fs/compat_binfmt_elf.c#L144
> > the cause of this behavior, which is unexpected but correct.
>
> Thank you for the investigation, this an interesting reading!
>
> > The rationale is that using source file + line number iproduces better
> > kallsyms table, but it is still do not produce unique names.
>
> When I first read your cover letter, I also thought it would be cool to have
> the filename rather than a serial ID.
> So, your latest modification is really good as it goes into this direction.
> Regarding having non unique names, I am not sure if this is a problem.
> In my case, which is using kprobe to attach BPF code to trace some events,
> your contribution would make me on the way to know which address I should use
> because I could choose regarding the filename.

This whole thing got trickier than I expected when I started. At first, I was
just grappling with different functions having the same name.
But during my investigation, it hit me that I've got to handle multiple copies
of the same function too.
So, those addresses labeled with the same name?
They're not just different functions having the same name - they're a mix of
functions that happen to share a name and functions that are the same but come
from different compile units.

Imagine this:
one compile unit defines a function foo1() that uses a bar() from a
shared header.
Another compile unit defines foo2(), also relying on bar().
The snag is, these compile units end up being linked together, carrying their
own copies of bar().
So, if I wanna keep an eye on bar(), which copy should I watch?
The answer depends on whose bar() I want to watch at.
I could go for probing them all, sure, but that's kind of a suboptimal
solution that waters down what I'm trying to achieve.

>
> I am curious about a potential v3 of this patch!

The way things are being done right now, even though it's nice that it can
track down these copies in the kernel image, it doesn't quite hit the
bullseye of what I was going for. But hey, I'm totally open to putting out
v3 with the limitations I just talked about, and discuss if it can be improved.

>
> > BR
> > Alessandro
> >
> > > As a final note, please understand that my patch was not intended to
> > > undermine anyone's work. I simply encountered a problem that I wanted to
> > > help solve. Attached to this message is my code, in case anyone wants to
> > > replicate it. I would appreciate being kept in the loop, as I genuinely
> > > want to assist in fixing this problem.
> > >
> > > BR
> > > Alessandro
> > > ---
> > >
> > >  init/Kconfig                        |  36 ++++
> > >  scripts/Makefile                    |   4 +
> > >  scripts/kas_alias/Makefile          |   4 +
> > >  scripts/kas_alias/a2l.c             | 268 ++++++++++++++++++++++++++++
> > >  scripts/kas_alias/a2l.h             |  32 ++++
> > >  scripts/kas_alias/duplicates_list.c |  70 ++++++++
> > >  scripts/kas_alias/duplicates_list.h |  15 ++
> > >  scripts/kas_alias/item_list.c       | 230 ++++++++++++++++++++++++
> > >  scripts/kas_alias/item_list.h       |  26 +++
> > >  scripts/kas_alias/kas_alias.c       | 217 ++++++++++++++++++++++
> > >  scripts/link-vmlinux.sh             |  11 +-
> > >  11 files changed, 910 insertions(+), 3 deletions(-)
> > >  create mode 100644 scripts/kas_alias/Makefile
> > >  create mode 100644 scripts/kas_alias/a2l.c
> > >  create mode 100644 scripts/kas_alias/a2l.h
> > >  create mode 100644 scripts/kas_alias/duplicates_list.c
> > >  create mode 100644 scripts/kas_alias/duplicates_list.h
> > >  create mode 100644 scripts/kas_alias/item_list.c
> > >  create mode 100644 scripts/kas_alias/item_list.h
> > >  create mode 100644 scripts/kas_alias/kas_alias.c
> > >
> > > diff --git a/init/Kconfig b/init/Kconfig
> > > index f7f65af4ee12..bc69fcd9cbc8 100644
> > > --- a/init/Kconfig
> > > +++ b/init/Kconfig
> > > @@ -1737,6 +1737,42 @@ config KALLSYMS_BASE_RELATIVE
> > >
> > >           time constants, and no relocation pass is required at runtime to
> > >           fix
> > >           up the entries based on the runtime load address of the kernel.
> > >
> > > +config KALLSYMS_ALIAS
> > > +       bool "Produces alias for duplicated symbols" if EXPERT
> > > +       depends on KALLSYMS && (DEBUG_INFO_DWARF4 || DEBUG_INFO_DWARF5)
> > > +       help
> > > +         It is not uncommon for drivers or modules related to similar
> > > +         peripherals to have symbols with the exact same name.
> > > +         While this is not a problem for the kernel's binary itself, it
> > > +         becomes an issue when attempting to trace or probe specific
> > > +         functions using infrastructure like ftrace or kprobe.
> > > +
> > > +         This option addresses this challenge by extending the symbol
> > > names +         with unique suffixes during the kernel build process.
> > > +         The newly created aliases for these duplicated symbols are
> > > unique
> > > +         names that can be fed to the ftrace sysfs interface. By doing
> > > so, it +         enables previously unreachable symbols to be probed.
> > > +
> > > +config CONFIG_KALLSYMS_ALIAS_DATA
> > > +       bool "Produces alias also for data"
> > > +       depends on KALLSYMS_ALIAS
> > > +       help
> > > +         Sometimes it can be useful to refer to data. In live patch
> > > scenarios, +         you may find yourself needing to use symbols that
> > > are shared with +         other functions. Since symbols face the same
> > > issue as functions, this +         option allows you to create aliases
> > > for data as well.
> > > +
> > > +config CONFIG_KALLSYMS_ALIAS_DATA_ALL
> > > +       bool "Removes all filter when producing data alias"
> > > +       depends on CONFIG_KALLSYMS_ALIAS_DATA
> > > +       help
> > > +         When selecting data aliases, not all symbols are included in the
> > > set +         This is because many symbols are unlikely to be used. If
> > > you choose +         to have an alias for all data symbols, be aware that
> > > it will +         significantly increase the size.
> > > +
> > > +         If unsure, say N.
> > > +
> > >
> > >  # end of the "standard kernel features (expert users)" menu
> > >
> > >  # syscall, maps, verifier
> > >
> > > diff --git a/scripts/Makefile b/scripts/Makefile
> > > index 32b6ba722728..65fafe17cfe5 100644
> > > --- a/scripts/Makefile
> > > +++ b/scripts/Makefile
> > > @@ -49,3 +49,7 @@ subdir-$(CONFIG_SECURITY_SELINUX) += selinux
> > >
> > >  # Let clean descend into subdirs
> > >  subdir-        += basic dtc gdb kconfig mod
> > >
> > > +
> > > +# KALLSyms alias
> > > +subdir-$(CONFIG_KALLSYMS_ALIAS) += kas_alias
> > > +
> > > diff --git a/scripts/kas_alias/Makefile b/scripts/kas_alias/Makefile
> > > new file mode 100644
> > > index 000000000000..e1fde69232b4
> > > --- /dev/null
> > > +++ b/scripts/kas_alias/Makefile
> > > @@ -0,0 +1,4 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +hostprogs-always-$(CONFIG_KALLSYMS_ALIAS)    += kas_alias
> > > +
> > > +kas_alias-objs        := duplicates_list.o item_list.o kas_alias.o a2l.o
> > > diff --git a/scripts/kas_alias/a2l.c b/scripts/kas_alias/a2l.c
> > > new file mode 100644
> > > index 000000000000..a9692ac30180
> > > --- /dev/null
> > > +++ b/scripts/kas_alias/a2l.c
> > > @@ -0,0 +1,268 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +#include <stdio.h>
> > > +#include <stdlib.h>
> > > +#include <string.h>
> > > +#include <unistd.h>
> > > +#include <sys/types.h>
> > > +#include <sys/wait.h>
> > > +#include <string.h>
> > > +#include <stdint.h>
> > > +#include <stdbool.h>
> > > +
> > > +#include "a2l.h"
> > > +
> > > +int addr2line_pid = -1;
> > > +int a2l_in[2];
> > > +int a2l_out[2];
> > > +char line[MAX_BUF];
> > > +char vmlinux_path[MAX_BUF];
> > > +char addr2line_cmd[MAX_CMD_LEN];
> > > +FILE *a2l_stdin, *a2l_stdout;
> > > +
> > > +static char *normalize_path(const char *input_path, char *output_path)
> > > +{
> > > +       char *prev_token = NULL;
> > > +       char *delimiter = "/";
> > > +       char inbuf[MAX_BUF];
> > > +       char *token;
> > > +       char *pos;
> > > +
> > > +       memset(inbuf, 0, MAX_BUF);
> > > +       *output_path = '\0';
> > > +       strncpy(inbuf, input_path, MAX_BUF);
> > > +       if (!input_path || !output_path || strlen(input_path) == 0)
> > > +               return NULL;
> > > +
> > > +       token = strtok(inbuf, delimiter);
> > > +       while (token) {
> > > +               if (strcmp(token, "..") == 0 && prev_token) {
> > > +                       pos = strrchr(output_path, '/');
> > > +                       if (pos)
> > > +                               *pos = '\0';
> > > +
> > > +               } else if (strcmp(token, ".") != 0) {
> > > +                       strcat(output_path, "/");
> > > +                       strcat(output_path, token);
> > > +               }
> > > +
> > > +               prev_token = token;
> > > +               token = strtok(NULL, delimiter);
> > > +       }
> > > +
> > > +       return output_path;
> > > +}
> > > +
> > > +static void path_of(const char *full_path, char *path)
> > > +{
> > > +       const char *last_slash = strrchr(full_path, '/');
> > > +       size_t path_length;
> > > +       char cwd[MAX_BUF];
> > > +
> > > +       if (!last_slash) {
> > > +               if (getcwd(cwd, sizeof(cwd)))
> > > +                       strcpy(path, cwd);
> > > +               else
> > > +                       strcpy(path, ".");
> > > +       } else {
> > > +               path_length = last_slash - full_path;
> > > +               strncpy(path, full_path, path_length);
> > > +               path[path_length] = '\0';
> > > +       }
> > > +}
> > > +
> > > +static bool file_exists(const char *file_path)
> > > +{
> > > +       FILE *file;
> > > +
> > > +       file = fopen(file_path, "r");
> > > +       if (file) {
> > > +               fclose(file);
> > > +               return true;
> > > +       }
> > > +       return false;
> > > +}
> > > +
> > > +int addr2line_init(const char *cmd, const char *vmlinux)
> > > +{
> > > +       if ((!file_exists(cmd)) || (!file_exists(vmlinux))) {
> > > +               printf("file not found\n");
> > > +               return 0;
> > > +               }
> > > +
> > > +       path_of(vmlinux, vmlinux_path);
> > > +       if (pipe(a2l_in) == -1) {
> > > +               printf("Failed to create pipe\n");
> > > +               return 0;
> > > +       }
> > > +
> > > +       if (pipe(a2l_out) == -1) {
> > > +               printf("Failed to create pipe\n");
> > > +               return 0;
> > > +       }
> > > +
> > > +       addr2line_pid = fork();
> > > +       if (addr2line_pid == -1) {
> > > +               printf("Failed to fork process\n");
> > > +               close(a2l_in[P_READ]);
> > > +               close(a2l_in[P_WRITE]);
> > > +               close(a2l_out[P_READ]);
> > > +               close(a2l_out[P_WRITE]);
> > > +               return 0;
> > > +       }
> > > +
> > > +       if (addr2line_pid == 0) {
> > > +               dup2(a2l_in[P_READ], 0);
> > > +               dup2(a2l_out[P_WRITE], 1);
> > > +               close(a2l_in[P_WRITE]);
> > > +               close(a2l_out[P_READ]);
> > > +
> > > +               execlp(cmd, cmd, ADDR2LINE_ARGS, vmlinux, NULL);
> > > +
> > > +               printf("Failed to execute addr2line command\n");
> > > +               exit(1);
> > > +       } else {
> > > +               close(a2l_in[P_READ]);
> > > +               close(a2l_out[P_WRITE]);
> > > +       }
> > > +
> > > +       a2l_stdin = fdopen(a2l_in[P_WRITE], "w");
> > > +       if (!a2l_stdin) {
> > > +               printf("Failed to open pipe a2l_in\n");
> > > +               return 0;
> > > +       }
> > > +
> > > +       a2l_stdout = fdopen(a2l_out[P_READ], "r");
> > > +       if (!a2l_stdout) {
> > > +               printf("Failed to open pipe a2l_out\n");
> > > +               fclose(a2l_stdin);
> > > +               return 0;
> > > +       }
> > > +
> > > +       return 1;
> > > +}
> > > +
> > > +const char *remove_subdir(const char *home, const char *f_path)
> > > +{
> > > +       int i = 0;
> > > +
> > > +       while (*(home + i) == *(f_path + i))
> > > +               i++;
> > > +
> > > +       return (strlen(home) != i) ? NULL : f_path + i;
> > > +}
> > > +
> > > +char *addr2line_get_lines(uint64_t address)
> > > +{
> > > +       char buf[MAX_BUF];
> > > +
> > > +       fprintf(a2l_stdin, "%08lx\n", address);
> > > +       fflush(a2l_stdin);
> > > +
> > > +       if (!fgets(line, sizeof(line), a2l_stdout)) {
> > > +               printf("Failed to read lines from addr2line\n");
> > > +               return NULL;
> > > +       }
> > > +
> > > +       if (!fgets(line, sizeof(line), a2l_stdout)) {
> > > +               printf("Failed to read lines from addr2line\n");
> > > +               return NULL;
> > > +       }
> > > +
> > > +       line[strcspn(line, "\n")] = '\0';
> > > +       strncpy(buf, line, MAX_BUF);
> > > +       return normalize_path(buf, line);
> > > +}
> > > +
> > > +int addr2line_cleanup(void)
> > > +{
> > > +       int status;
> > > +
> > > +       if (addr2line_pid != -1) {
> > > +               kill(addr2line_pid, SIGKILL);
> > > +               waitpid(addr2line_pid, &status, 0);
> > > +               fclose(a2l_stdin);
> > > +               fclose(a2l_stdout);
> > > +               addr2line_pid = -1;
> > > +       }
> > > +
> > > +       return 1;
> > > +}
> > > +
> > > +static char *find_executable(const char *command)
> > > +{
> > > +       char *path_env = getenv("PATH");
> > > +       char *executable_path;
> > > +       char *path_copy;
> > > +       char *path;
> > > +       int n;
> > > +
> > > +       if (!path_env)
> > > +               return NULL;
> > > +
> > > +       path_copy = strdup(path_env);
> > > +       if (!path_copy)
> > > +               return NULL;
> > > +
> > > +       path = strtok(path_copy, ":");
> > > +       while (path) {
> > > +               n = snprintf(0, 0, "%s/%s", path, command);
> > > +               executable_path = (char *)malloc(n + 1);
> > > +               snprintf(executable_path, n + 1, "%s/%s", path, command);
> > > +               if (access(executable_path, X_OK) == 0) {
> > > +                       free(path_copy);
> > > +                       return executable_path;
> > > +               }
> > > +
> > > +       path = strtok(NULL, ":");
> > > +       free(executable_path);
> > > +       executable_path = NULL;
> > > +       }
> > > +
> > > +       free(path_copy);
> > > +       if (executable_path)
> > > +               free(executable_path);
> > > +       return NULL;
> > > +}
> > > +
> > > +const char *get_addr2line(int mode)
> > > +{
> > > +       char *buf = "";
> > > +
> > > +       switch (mode) {
> > > +       case A2L_CROSS:
> > > +               buf = getenv("CROSS_COMPILE");
> > > +               memcpy(addr2line_cmd, buf, strlen(buf));
> > > +       case A2L_DEFAULT:
> > > +               memcpy(addr2line_cmd + strlen(buf), ADDR2LINE,
> > > strlen(ADDR2LINE)); +               buf = find_executable(addr2line_cmd);
> > > +               if (buf) {
> > > +                       memcpy(addr2line_cmd, buf, strlen(buf));
> > > +                       free(buf);
> > > +               }
> > > +               return addr2line_cmd;
> > > +       case A2L_LLVM:
> > > +       default:
> > > +               return NULL;
> > > +       }
> > > +}
> > > +
> > > +char *get_vmlinux(char *input)
> > > +{
> > > +       const char *match_string1 = ".syms";
> > > +       const char *match_string2 = ".tmp_vmlinux.kallsyms";
> > > +       char *result = NULL;
> > > +       char *match_pos;
> > > +
> > > +       match_pos = strstr(input, match_string1);
> > > +       if (!match_pos)
> > > +               return NULL;
> > > +
> > > +       match_pos = strstr(input, match_string2);
> > > +       if (!match_pos)
> > > +               return NULL;
> > > +
> > > +       result = strdup(input);
> > > +       match_pos = strstr(result, match_string1);
> > > +       *match_pos = '\0';
> > > +       return result;
> > > +}
> > > diff --git a/scripts/kas_alias/a2l.h b/scripts/kas_alias/a2l.h
> > > new file mode 100644
> > > index 000000000000..ca6419229dde
> > > --- /dev/null
> > > +++ b/scripts/kas_alias/a2l.h
> > > @@ -0,0 +1,32 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > +#ifndef A2L_H
> > > +#define A2L_H
> > > +#include <stdint.h>
> > > +
> > > +#define ADDR2LINE "addr2line"
> > > +#define ADDR2LINE_ARGS "-fe"
> > > +//#define VMLINUX "vmlinux"
> > > +#define MAX_BUF 4096
> > > +#define MAX_CMD_LEN 256
> > > +#define P_READ 0
> > > +#define P_WRITE 1
> > > +#define A2L_DEFAULT 1
> > > +#define A2L_CROSS 2
> > > +#define A2L_LLVM 3
> > > +#define A2L_MAKE_VALUE 2
> > > +
> > > +extern int addr2line_pid;
> > > +extern int a2l_in[2];
> > > +extern int a2l_out[2];
> > > +extern char line[MAX_BUF];
> > > +extern char vmlinux_path[MAX_BUF];
> > > +extern char addr2line_cmd[MAX_CMD_LEN];
> > > +
> > > +int addr2line_init(const char *cmd, const char *vmlinux);
> > > +char *addr2line_get_lines(uint64_t address);
> > > +int addr2line_cleanup(void);
> > > +const char *remove_subdir(const char *home, const char *f_path);
> > > +const char *get_addr2line(int mode);
> > > +char *get_vmlinux(char *input);
> > > +
> > > +#endif
> > > diff --git a/scripts/kas_alias/duplicates_list.c
> > > b/scripts/kas_alias/duplicates_list.c new file mode 100644
> > > index 000000000000..e7a3d2917937
> > > --- /dev/null
> > > +++ b/scripts/kas_alias/duplicates_list.c
> > > @@ -0,0 +1,70 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +#include <stdint.h>
> > > +#include <stdio.h>
> > > +#include <string.h>
> > > +#include <stdlib.h>
> > > +#include <stdbool.h>
> > > +
> > > +#include "item_list.h"
> > > +#include "duplicates_list.h"
> > > +
> > > +struct duplicate_item *find_duplicates(struct item *list)
> > > +{
> > > +       struct duplicate_item *current_duplicate = NULL;
> > > +       struct duplicate_item *duplicates = NULL;
> > > +       struct duplicate_item *new_duplicate;
> > > +       struct item *current_item = list;
> > > +       bool prev_was_duplicate = false;
> > > +       struct item *prev_item = NULL;
> > > +
> > > +       while (current_item) {
> > > +               if ((prev_item && (strcmp(current_item->symb_name,
> > > prev_item->symb_name) == 0)) || +                   prev_was_duplicate) {
> > > +                       if (!duplicates) {
> > > +                               duplicates = malloc(sizeof(struct
> > > duplicate_item)); +                               if (!duplicates)
> > > +                                       return NULL;
> > > +
> > > +                               duplicates->original_item = prev_item;
> > > +                               duplicates->next = NULL;
> > > +                               current_duplicate = duplicates;
> > > +                       } else {
> > > +                               new_duplicate = malloc(sizeof(struct
> > > duplicate_item)); +                               if (!new_duplicate) {
> > > +                                       free_duplicates(&duplicates);
> > > +                                       return NULL;
> > > +                               }
> > > +
> > > +                               new_duplicate->original_item = prev_item;
> > > +                               new_duplicate->next = NULL;
> > > +                               current_duplicate->next = new_duplicate;
> > > +                               current_duplicate = new_duplicate;
> > > +
> > > +                               if ((strcmp(current_item->symb_name,
> > > prev_item->symb_name) != 0) && +
> > > (prev_was_duplicate))
> > > +                                       prev_was_duplicate = false;
> > > +                               else
> > > +                                       prev_was_duplicate = true;
> > > +                       }
> > > +               }
> > > +
> > > +               prev_item = current_item;
> > > +               current_item = current_item->next;
> > > +       }
> > > +
> > > +       return duplicates;
> > > +}
> > > +
> > > +void free_duplicates(struct duplicate_item **duplicates)
> > > +{
> > > +       struct duplicate_item *duplicates_iterator = *duplicates;
> > > +       struct duplicate_item *app;
> > > +
> > > +       while (duplicates_iterator) {
> > > +               app = duplicates_iterator;
> > > +               duplicates_iterator = duplicates_iterator->next;
> > > +               free(app);
> > > +       }
> > > +
> > > +       *duplicates = NULL;
> > > +}
> > > diff --git a/scripts/kas_alias/duplicates_list.h
> > > b/scripts/kas_alias/duplicates_list.h new file mode 100644
> > > index 000000000000..76aa73e584bc
> > > --- /dev/null
> > > +++ b/scripts/kas_alias/duplicates_list.h
> > > @@ -0,0 +1,15 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > +#ifndef DUPLICATES_LIST_H
> > > +#define DUPLICATES_LIST_H
> > > +
> > > +#include "item_list.h"
> > > +
> > > +struct duplicate_item {
> > > +       struct item *original_item;
> > > +       struct duplicate_item *next;
> > > +};
> > > +
> > > +struct duplicate_item *find_duplicates(struct item *list);
> > > +void free_duplicates(struct duplicate_item **duplicates);
> > > +
> > > +#endif
> > > diff --git a/scripts/kas_alias/item_list.c b/scripts/kas_alias/item_list.c
> > > new file mode 100644
> > > index 000000000000..48f2e525592a
> > > --- /dev/null
> > > +++ b/scripts/kas_alias/item_list.c
> > > @@ -0,0 +1,230 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +#include <stdio.h>
> > > +#include <stdlib.h>
> > > +#include <stdint.h>
> > > +#include <string.h>
> > > +#include <stdbool.h>
> > > +#include <assert.h>
> > > +#include "item_list.h"
> > > +
> > > +#define CHECK_ORDER_BY_ADDRESS(sort_by, current, temp, op) \
> > > +       ((sort_by) == BY_ADDRESS && (current)->addr op (temp)->addr)
> > > +#define CHECK_ORDER_BY_NAME(sort_by, current, temp, op) \
> > > +       ((sort_by) == BY_NAME && strcmp((current)->symb_name,
> > > (temp)->symb_name) op 0) +
> > > +struct item *list_index[96] = {0};
> > > +
> > > +void build_index(struct item *list)
> > > +{
> > > +       char current_first_letter = ' ';
> > > +       struct item *current = list;
> > > +
> > > +       while (current) {
> > > +               if (current->symb_name[0] != current_first_letter) {
> > > +                       current_first_letter = current->symb_name[0];
> > > +                       list_index[current_first_letter - 32] = current;
> > > +               }
> > > +               current = current->next;
> > > +       }
> > > +}
> > > +
> > > +struct item *add_item(struct item **list, const char *name, char stype,
> > > uint64_t addr) +{
> > > +       struct item *new_item;
> > > +       struct item *current;
> > > +
> > > +       new_item = malloc(sizeof(struct item));
> > > +       if (!new_item)
> > > +               return NULL;
> > > +
> > > +       strncpy(new_item->symb_name, name, MAX_NAME_SIZE);
> > > +       new_item->symb_name[MAX_NAME_SIZE - 1] = '\0';
> > > +       new_item->addr = addr;
> > > +       new_item->stype = stype;
> > > +       new_item->next = NULL;
> > > +
> > > +       if (!(*list)) {
> > > +               *list = new_item;
> > > +       } else {
> > > +               current = *list;
> > > +               while (current->next)
> > > +                       current = current->next;
> > > +
> > > +               current->next = new_item;
> > > +       }
> > > +       return new_item;
> > > +}
> > > +
> > > +void sort_list(struct item **list, int sort_by)
> > > +{
> > > +       struct item *current = *list;
> > > +       struct item *sorted = NULL;
> > > +       struct item *next_item;
> > > +       struct item *temp;
> > > +
> > > +       if (!(*list) || !((*list)->next))
> > > +               return;
> > > +
> > > +       while (current) {
> > > +               next_item = current->next;
> > > +               if (!sorted ||
> > > +                   (CHECK_ORDER_BY_ADDRESS(sort_by, current, sorted, <)
> > > ||
> > > +                   CHECK_ORDER_BY_NAME(sort_by, current, sorted, >=))) {
> > > +                       current->next = sorted;
> > > +                       sorted = current;
> > > +               } else {
> > > +                       temp = sorted;
> > > +                       while (temp->next &&
> > > +                              (CHECK_ORDER_BY_ADDRESS(sort_by, current,
> > > temp->next, >=) || +
> > > CHECK_ORDER_BY_NAME(sort_by, current, temp->next, >=))) +
> > >               temp = temp->next;
> > > +
> > > +                       current->next = temp->next;
> > > +                       temp->next = current;
> > > +               }
> > > +               current = next_item;
> > > +       }
> > > +
> > > +       *list = sorted;
> > > +}
> > > +
> > > +struct item *merge(struct item *left, struct item *right, int sort_by)
> > > +{
> > > +       struct item *current = NULL;
> > > +       struct item *result = NULL;
> > > +
> > > +       if (!left)
> > > +               return right;
> > > +       if (!right)
> > > +               return left;
> > > +
> > > +       if (sort_by == BY_NAME) {
> > > +               if (strcmp(left->symb_name, right->symb_name) <= 0) {
> > > +                       result = left;
> > > +                       left = left->next;
> > > +               } else {
> > > +                       result = right;
> > > +                       right = right->next;
> > > +               }
> > > +       } else {
> > > +               if (sort_by == BY_ADDRESS) {
> > > +                       if (left->addr <= right->addr) {
> > > +                               result = left;
> > > +                               left = left->next;
> > > +                       } else {
> > > +                               result = right;
> > > +                               right = right->next;
> > > +                       }
> > > +               }
> > > +       }
> > > +
> > > +       current = result;
> > > +
> > > +       while (left && right) {
> > > +               if (sort_by == BY_NAME) {
> > > +                       if (strcmp(left->symb_name, right->symb_name) <=
> > > 0) { +                               current->next = left;
> > > +                               left = left->next;
> > > +                       } else {
> > > +                               current->next = right;
> > > +                               right = right->next;
> > > +                       }
> > > +               } else {
> > > +                       if (sort_by == BY_ADDRESS) {
> > > +                               if (left->addr <= right->addr) {
> > > +                                       current->next = left;
> > > +                                       left = left->next;
> > > +                               } else {
> > > +                                       current->next = right;
> > > +                                       right = right->next;
> > > +                               }
> > > +                       }
> > > +               }
> > > +
> > > +               current = current->next;
> > > +       }
> > > +
> > > +       if (left) {
> > > +               current->next = left;
> > > +       } else {
> > > +               if (right)
> > > +                       current->next = right;
> > > +       }
> > > +
> > > +       return result;
> > > +}
> > > +
> > > +struct item *merge_sort(struct item *head, int sort_by)
> > > +{
> > > +       struct item *right;
> > > +       struct item *slow;
> > > +       struct item *fast;
> > > +       struct item *left;
> > > +
> > > +       if (!head || !head->next)
> > > +               return head;
> > > +
> > > +       slow = head;
> > > +       fast = head->next;
> > > +
> > > +       while (fast && fast->next) {
> > > +               slow = slow->next;
> > > +               fast = fast->next->next;
> > > +       }
> > > +
> > > +       left = head;
> > > +       right = slow->next;
> > > +       slow->next = NULL;
> > > +
> > > +       left = merge_sort(left, sort_by);
> > > +       right = merge_sort(right, sort_by);
> > > +
> > > +       return merge(left, right, sort_by);
> > > +}
> > > +
> > > +void sort_list_m(struct item **head, int sort_by)
> > > +{
> > > +       if (!(*head) || !((*head)->next))
> > > +               return;
> > > +
> > > +       *head = merge_sort(*head, sort_by);
> > > +}
> > > +
> > > +int insert_after(struct item *list, const uint64_t search_addr,
> > > +                const char *name, uint64_t addr, char stype)
> > > +{
> > > +       struct item *new_item;
> > > +       struct item *current;
> > > +       int ret = 0;
> > > +
> > > +       current = (list_index[name[0] - 32]) ? list_index[name[0] - 32] :
> > > list; +       while (current) {
> > > +               if (current->addr == search_addr) {
> > > +                       new_item = malloc(sizeof(struct item));
> > > +                       if (!new_item)
> > > +                               return ret;
> > > +                       strncpy(new_item->symb_name, name, MAX_NAME_SIZE);
> > > +                       new_item->symb_name[MAX_NAME_SIZE - 1] = '\0';
> > > +                       new_item->addr = addr;
> > > +                       new_item->stype = stype;
> > > +                       new_item->next = current->next;
> > > +                       current->next = new_item;
> > > +                       ret = 1;
> > > +                       break;
> > > +               }
> > > +               current = current->next;
> > > +       }
> > > +       return ret;
> > > +}
> > > +
> > > +void free_items(struct item **head)
> > > +{
> > > +       struct item *app, *item_iterator = *head;
> > > +
> > > +       while (item_iterator) {
> > > +               app = item_iterator;
> > > +               item_iterator = item_iterator->next;
> > > +               free(app);
> > > +       }
> > > +       *head = NULL;
> > > +}
> > > diff --git a/scripts/kas_alias/item_list.h b/scripts/kas_alias/item_list.h
> > > new file mode 100644
> > > index 000000000000..b4891cb088ee
> > > --- /dev/null
> > > +++ b/scripts/kas_alias/item_list.h
> > > @@ -0,0 +1,26 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > +#ifndef ITEM_LIST_H
> > > +#define ITEM_LIST_H
> > > +#include <stdint.h>
> > > +
> > > +#define MAX_NAME_SIZE 256
> > > +#define BY_ADDRESS 1
> > > +#define BY_NAME 2
> > > +
> > > +struct item {
> > > +       char            symb_name[MAX_NAME_SIZE];
> > > +       uint64_t        addr;
> > > +       char            stype;
> > > +       struct item     *next;
> > > +};
> > > +
> > > +void build_index(struct item *list);
> > > +struct item *add_item(struct item **list, const char *name, char stype,
> > > uint64_t addr); +void sort_list(struct item **list, int sort_by);
> > > +struct item *merge(struct item *left, struct item *right, int sort_by);
> > > +struct item *merge_sort(struct item *head, int sort_by);
> > > +void sort_list_m(struct item **head, int sort_by);
> > > +int insert_after(struct item *list, const uint64_t search_addr,
> > > +                const char *name, uint64_t addr, char stype);
> > > +void free_items(struct item **head);
> > > +#endif
> > > diff --git a/scripts/kas_alias/kas_alias.c b/scripts/kas_alias/kas_alias.c
> > > new file mode 100644
> > > index 000000000000..532aeb39f851
> > > --- /dev/null
> > > +++ b/scripts/kas_alias/kas_alias.c
> > > @@ -0,0 +1,217 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +#include <stdio.h>
> > > +#include <stdlib.h>
> > > +#include <stdint.h>
> > > +#include <unistd.h>
> > > +#include <string.h>
> > > +#include <stdbool.h>
> > > +#include <stdarg.h>
> > > +#include <regex.h>
> > > +
> > > +#include "item_list.h"
> > > +#include "duplicates_list.h"
> > > +#include "a2l.h"
> > > +
> > > +#define SYMB_IS_TEXT(s) ((((s)->stype) == 't') ||  (((s)->stype) == 'T'))
> > > +#define SYMB_IS_DATA(s) ((((s)->stype) == 'b') ||  (((s)->stype) == 'B')
> > > || \ +                        (((s)->stype) == 'd') ||  (((s)->stype) ==
> > > 'D') || \ +                        (((s)->stype) == 'r') ||
> > > (((s)->stype) == 'R')) +#ifdef CONFIG_KALLSYMS_ALIAS_DATA
> > > +#define SYMB_NEEDS_ALIAS(s) (SYMB_IS_TEXT(s) || SYMB_IS_DATA(s))
> > > +#else
> > > +#define SYMB_NEEDS_ALIAS(s) SYMB_IS_TEXT(s)
> > > +#endif
> > > +#define FNOMATCH 0
> > > +#define FMATCH 1
> > > +#define EREGEX 2
> > > +
> > > +const char *ignore_regex[] = {
> > > +       "^__cfi_.*$",                           // __cfi_ preamble
> > > +#ifndef CONFIG_KALLSYMS_ALIAS_DATA_ALL
> > > +       "^_*TRACE_SYSTEM.*$",
> > > +       "^__already_done\\.[0-9]+$",            // Call a function once
> > > data +       "^___tp_str\\.[0-9]+$",
> > > +       "^___done\\.[0-9]+$",
> > > +       "^__print_once\\.[0-9]+$",
> > > +       "^_rs\\.[0-9]+$",
> > > +       "^__compound_literal\\.[0-9]+$",
> > > +       "^___once_key\\.[0-9]+$",
> > > +       "^__func__\\.[0-9]+$",
> > > +       "^__msg\\.[0-9]+$",
> > > +       "^CSWTCH\\.[0-9]+$",
> > > +       "^__flags\\.[0-9]+$",
> > > +       "^__wkey.*$",
> > > +       "^__mkey.*$",
> > > +       "^__key.*$",
> > > +#endif
> > > +       "^__pfx_.*$"                            // NOP-padding
> > > +};
> > > +
> > > +int suffix_serial;
> > > +
> > > +static inline void verbose_msg(bool verbose, const char *fmt, ...)
> > > +{
> > > +       va_list args;
> > > +
> > > +       va_start(args, fmt);
> > > +       if (verbose)
> > > +               printf(fmt, args);
> > > +
> > > +       va_end(args);
> > > +}
> > > +
> > > +static void create_suffix(const char *name, char *output_suffix)
> > > +{
> > > +       sprintf(output_suffix, "%s__alias__%d", name, suffix_serial++);
> > > +}
> > > +
> > > +static void create_file_suffix(const char *name, uint64_t address, char
> > > *output_suffix, char *cwd) +{
> > > +       const char *f_path;
> > > +       char *buf;
> > > +       int i = 0;
> > > +
> > > +       buf = addr2line_get_lines(address);
> > > +       f_path = remove_subdir(cwd, buf);
> > > +       if (f_path) {
> > > +               sprintf(output_suffix, "%s@%s", name, f_path);
> > > +               while (*(output_suffix + i) != '\0') {
> > > +                       switch (*(output_suffix + i)) {
> > > +                       case '/':
> > > +                       case ':':
> > > +                       case '.':
> > > +                               *(output_suffix + i) = '_';
> > > +                               break;
> > > +                       default:
> > > +                       }
> > > +               i++;
> > > +               }
> > > +       } else {
> > > +               create_suffix(name, output_suffix);
> > > +       }
> > > +}
> > > +
> > > +static int filter_symbols(char *symbol, const char **ignore_list, int
> > > regex_no) +{
> > > +       regex_t regex;
> > > +       int res, i;
> > > +
> > > +       for (i = 0; i < regex_no; i++) {
> > > +               res = regcomp(&regex, ignore_list[i], REG_EXTENDED);
> > > +               if (res)
> > > +                       return -EREGEX;
> > > +
> > > +               res = regexec(&regex, symbol, 0, NULL, 0);
> > > +               regfree(&regex);
> > > +               switch (res) {
> > > +               case 0:
> > > +                       return FMATCH;
> > > +               case REG_NOMATCH:
> > > +                       break;
> > > +               default:
> > > +                       return -EREGEX;
> > > +               }
> > > +       }
> > > +
> > > +       return FNOMATCH;
> > > +}
> > > +
> > > +int main(int argc, char *argv[])
> > > +{
> > > +       char t, sym_name[MAX_NAME_SIZE], new_name[MAX_NAME_SIZE + 15];
> > > +       struct duplicate_item  *duplicate_iterator;
> > > +       struct duplicate_item *duplicate;
> > > +       struct item *head = {NULL};
> > > +       bool need_2_process = true;
> > > +       struct item *last = {NULL};
> > > +       struct item  *current;
> > > +       int verbose_mode = 0;
> > > +       uint64_t address;
> > > +       FILE *fp;
> > > +       int res;
> > > +
> > > +       if (argc < 2 || argc > 3) {
> > > +               printf("Usage: %s <nmfile> [-verbose]\n", argv[0]);
> > > +               return 1;
> > > +       }
> > > +
> > > +       if (argc == 3 && strcmp(argv[2], "-verbose") == 0)
> > > +               verbose_mode = 1;
> > > +
> > > +       verbose_msg(verbose_mode, "Scanning nm data(%s)\n", argv[1]);
> > > +
> > > +       fp = fopen(argv[1], "r");
> > > +       if (!fp) {
> > > +               printf("Can't open input file.\n");
> > > +               return 1;
> > > +       }
> > > +
> > > +       if (!addr2line_init(get_addr2line(A2L_DEFAULT),
> > > get_vmlinux(argv[1]))) +               return 1;
> > > +
> > > +       while (fscanf(fp, "%lx %c %99s\n", &address, &t, sym_name) == 3) {
> > > +               if (strstr(sym_name, "@_")) {
> > > +                       if (verbose_mode && need_2_process)
> > > +                               printf("Already processed\n");
> > > +                       need_2_process = false;
> > > +                       }
> > > +               last = add_item(&last, sym_name, t, address);
> > > +               if (!last) {
> > > +                       printf("Error in allocate memory\n");
> > > +                       free_items(&head);
> > > +                       return 1;
> > > +               }
> > > +
> > > +               if (!head)
> > > +                       head = last;
> > > +       }
> > > +
> > > +       fclose(fp);
> > > +
> > > +       if (need_2_process) {
> > > +               verbose_msg(verbose_mode, "Sorting nm data\n");
> > > +               sort_list_m(&head, BY_NAME);
> > > +               verbose_msg(verbose_mode, "Scanning nm data for
> > > duplicates\n"); +               duplicate = find_duplicates(head);
> > > +               if (!duplicate) {
> > > +                       printf("Error in duplicates list\n");
> > > +                       return 1;
> > > +               }
> > > +
> > > +               verbose_msg(verbose_mode, "Applying suffixes\n");
> > > +               build_index(head);
> > > +               duplicate_iterator = duplicate;
> > > +               while (duplicate_iterator) {
> > > +                       res =
> > > filter_symbols(duplicate_iterator->original_item->symb_name, +
> > >                                 ignore_regex, sizeof(ignore_regex) / +
> > >                                         sizeof(ignore_regex[0])); +
> > >                 if (res != FMATCH &&
> > > +
> > > SYMB_NEEDS_ALIAS(duplicate_iterator->original_item)) { +
> > >              if (res < 0)
> > > +                                       return 1;
> > > +
> > > +
> > > create_file_suffix(duplicate_iterator->original_item->symb_name, +
> > >
> > > duplicate_iterator->original_item->addr, +
> > >                   new_name, vmlinux_path); +
> > >  if (!insert_after(head, duplicate_iterator->original_item->addr, +
> > >                                           new_name,
> > > duplicate_iterator->original_item->addr, +
> > >                  duplicate_iterator->original_item->stype)) +
> > >                           return 1;
> > > +                       }
> > > +
> > > +                       duplicate_iterator = duplicate_iterator->next;
> > > +               }
> > > +
> > > +               sort_list_m(&head, BY_ADDRESS);
> > > +       }
> > > +       current = head;
> > > +       while (current) {
> > > +               printf("%08lx %c %s\n", current->addr, current->stype,
> > > current->symb_name); +               current = current->next;
> > > +       }
> > > +
> > > +       free_items(&head);
> > > +       free_duplicates(&duplicate);
> > > +       addr2line_cleanup();
> > > +       return 0;
> > > +}
> > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > > index a432b171be82..cacf60b597ce 100755
> > > --- a/scripts/link-vmlinux.sh
> > > +++ b/scripts/link-vmlinux.sh
> > > @@ -89,8 +89,9 @@ vmlinux_link()
> > >
> > >         ldflags="${ldflags} ${wl}--script=${objtree}/${KBUILD_LDS}"
> > >
> > > -       # The kallsyms linking does not need debug symbols included.
> > > -       if [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
> > > +       # The kallsyms linking does not need debug symbols included,
> > > unless the KALLSYMS_ALIAS. +       if [ ! is_enabled
> > > CONFIG_KALLSYMS_ALIAS ] && \
> > > +           [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
> > >
> > >                 ldflags="${ldflags} ${wl}--strip-debug"
> > >
> > >         fi
> > >
> > > @@ -161,7 +162,11 @@ kallsyms()
> > >
> > >         fi
> > >
> > >         info KSYMS ${2}
> > >
> > > -       scripts/kallsyms ${kallsymopt} ${1} > ${2}
> > > +       if is_enabled CONFIG_KALLSYMS_ALIAS; then
> > > +               ALIAS=".alias"
> > > +               scripts/kas_alias/kas_alias ${1} >${1}${ALIAS}
> > > +               fi
> > > +       scripts/kallsyms ${kallsymopt} ${1}${ALIAS} > ${2}
> > >
> > >  }
> > >
> > >  # Perform one step in kallsyms generation, including temporary linking of
> > >
> > > --
> > > 2.34.1
>
> Best regards.
>
>
BR
Alessandro

^ permalink raw reply

* Re: [PATCH RESEND v3 1/3] tracing/synthetic: use union instead of casts
From: kernel test robot @ 2023-08-25  8:26 UTC (permalink / raw)
  To: Sven Schnelle
  Cc: oe-lkp, lkp, linux-kernel, linux-trace-kernel, Steven Rostedt,
	Masami Hiramatsu, bpf, oliver.sang
In-Reply-To: <20230816154928.4171614-2-svens@linux.ibm.com>



Hello,

kernel test robot noticed "BUG:unable_to_handle_page_fault_for_address" on:

commit: e2c9745169808b98f235c09d1366cf8b53ce0d3c ("[PATCH RESEND v3 1/3] tracing/synthetic: use union instead of casts")
url: https://github.com/intel-lab-lkp/linux/commits/Sven-Schnelle/tracing-synthetic-use-union-instead-of-casts/20230817-002758
base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 4853c74bd7ab7fdb83f319bd9ace8a08c031e9b6
patch link: https://lore.kernel.org/all/20230816154928.4171614-2-svens@linux.ibm.com/
patch subject: [PATCH RESEND v3 1/3] tracing/synthetic: use union instead of casts

in testcase: boot

compiler: clang-16
test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 4G

(please refer to attached dmesg/kmsg for entire log/backtrace)



If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202308251530.5bd302e0-oliver.sang@intel.com



[   64.204176][   T48] Dumping ftrace buffer:
[   64.204632][   T48] ---------------------------------
[   64.205082][   T48] BUG: unable to handle page fault for address: 001c24ca
[   64.205641][   T48] #PF: supervisor read access in kernel mode
[   64.206115][   T48] #PF: error_code(0x0000) - not-present page
[   64.206588][   T48] *pde = 00000000
[   64.206897][   T48] Oops: 0000 [#1] SMP
[   64.207221][   T48] CPU: 0 PID: 48 Comm: rcu_scale_write Tainted: G                T  6.5.0-rc6-00037-ge2c974516980 #1
[   64.208074][   T48] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[   64.208892][   T48] EIP: string+0x98/0x100
[   64.209237][   T48] Code: 94 c2 75 b4 a1 8c c2 75 b4 40 89 44 24 18 31 d2 eb 0e 47 89 3d 94 c2 75 b4 42 39 54 24 0c 74 32 8b 44 24 08 8d 0c 10 8b 45 08 <0f> b6 04
 10 84 c0 74 2c 8b 74 24 18 01 d6 89 35 8c c2 75 b4 3b 0c
[   64.210761][   T48] EAX: 001c24ca EBX: 000405c2 ECX: b516e406 EDX: 00000000
[   64.211325][   T48] ESI: ffff0a00 EDI: 00005af2 EBP: b600bcd4 ESP: b600bca8
[   64.211876][   T48] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010046
[   64.212468][   T48] CR0: 80050033 CR2: 001c24ca CR3: 08d2a000 CR4: 000406d0
[   64.213022][   T48] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[   64.213577][   T48] DR6: fffe0ff0 DR7: 00000400
[   64.213945][   T48] Call Trace:
[   64.214208][   T48]  ? __die_body+0x69/0xc0
[   64.214546][   T48]  ? __die+0x7e/0x90
[   64.214863][   T48]  ? page_fault_oops+0x22d/0x2b0
[   64.215242][   T48]  ? is_prefetch+0x4a/0x220
[   64.215605][   T48]  ? kernelmode_fixup_or_oops+0xfa/0x140
[   64.216051][   T48]  ? __bad_area_nosemaphore+0x64/0x2a0
[   64.216485][   T48]  ? bad_area_nosemaphore+0x12/0x20
[   64.216899][   T48]  ? do_user_addr_fault+0x650/0x7e0
[   64.217313][   T48]  ? pvclock_clocksource_read_nowd+0x7c/0x230
[   64.217800][   T48]  ? exc_page_fault+0xc5/0x358
[   64.218183][   T48]  ? pvclock_clocksource_read_nowd+0x230/0x230
[   64.218666][   T48]  ? handle_exception+0x14b/0x14b
[   64.219059][   T48]  ? number+0x9b/0x630
[   64.219378][   T48]  ? pvclock_clocksource_read_nowd+0x230/0x230
[   64.219866][   T48]  ? string+0x98/0x100
[   64.220187][   T48]  ? pvclock_clocksource_read_nowd+0x230/0x230
[   64.220656][   T48]  ? string+0x98/0x100
[   64.220986][   T48]  vsnprintf+0x420/0x580
[   64.221323][   T48]  ? vsnprintf+0x3e7/0x580
[   64.221669][   T48]  seq_buf_vprintf+0x79/0xc0
[   64.222029][   T48]  trace_seq_printf+0x35/0xa0
[   64.222400][   T48]  print_synth_event+0x26f/0x300
[   64.222805][   T48]  ? trace_event_raw_event_synth+0x410/0x410
[   64.223272][   T48]  print_trace_fmt+0xfe/0x170
[   64.223651][   T48]  print_trace_line+0x10d/0x1c0
[   64.224046][   T48]  ftrace_dump+0x2fa/0x410
[   64.224407][   T48]  rcu_scale_writer+0x59c/0x640
[   64.224808][   T48]  kthread+0x158/0x170
[   64.225146][   T48]  ? rcu_scale_reader+0x180/0x180
[   64.225559][   T48]  ? kthread_unuse_mm+0x160/0x160
[   64.225968][   T48]  ? kthread_unuse_mm+0x160/0x160
[   64.226379][   T48]  ret_from_fork+0x43/0x70
[   64.226742][   T48]  ret_from_fork_asm+0x12/0x1c
[   64.227130][   T48]  entry_INT80_32+0x108/0x108
[   64.227516][   T48] Modules linked in:
[   64.227832][   T48] CR2: 00000000001c24ca
[   64.228166][   T48] ---[ end trace 0000000000000000 ]---
[   64.228586][   T48] EIP: string+0x98/0x100
[   64.228921][   T48] Code: 94 c2 75 b4 a1 8c c2 75 b4 40 89 44 24 18 31 d2 eb 0e 47 89 3d 94 c2 75 b4 42 39 54 24 0c 74 32 8b 44 24 08 8d 0c 10 8b 45 08 <0f> b6 04
 10 84 c0 74 2c 8b 74 24 18 01 d6 89 35 8c c2 75 b4 3b 0c
[   64.230401][   T48] EAX: 001c24ca EBX: 000405c2 ECX: b516e406 EDX: 00000000
[   64.230918][   T48] ESI: ffff0a00 EDI: 00005af2 EBP: b600bcd4 ESP: b600bca8
[   64.231432][   T48] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010046
[   64.231993][   T48] CR0: 80050033 CR2: 001c24ca CR3: 08d2a000 CR4: 000406d0
[   64.232527][   T48] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[   64.233094][   T48] DR6: fffe0ff0 DR7: 00000400
[   64.233474][   T48] Kernel panic - not syncing: Fatal exception
[   64.234050][   T48] Kernel Offset: disabled



The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20230825/202308251530.5bd302e0-oliver.sang@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply

* Re: (subset) [PATCH 00/17] -Wmissing-prototype warning fixes
From: Geert Uytterhoeven @ 2023-08-25  7:39 UTC (permalink / raw)
  To: Michael Schmitz
  Cc: Martin K. Petersen, Andrew Morton, linux-kernel, Arnd Bergmann,
	Arnd Bergmann, Matt Turner, Vineet Gupta, Russell King,
	Catalin Marinas, Will Deacon, Guo Ren, Brian Cain, Huacai Chen,
	WANG Xuerui, Michal Simek, Thomas Bogendoerfer, Dinh Nguyen,
	Jonas Bonn, Stefan Kristiansson, Stafford Horne,
	James E.J. Bottomley, Helge Deller, Michael Ellerman,
	Christophe Leroy, Palmer Dabbelt, Heiko Carstens,
	John Paul Adrian Glaubitz, x86, Borislav Petkov, Max Filippov,
	Jens Axboe, Sudip Mukherjee, Richard Weinberger, Bjorn Helgaas,
	Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Guenter Roeck, Stephen Rothwell, linux-next, linux-alpha,
	linux-snps-arc, linux-arm-kernel, linux-csky, linux-hexagon,
	linux-ia64, loongarch, linux-m68k, linux-mips, linux-openrisc,
	linux-parisc, linuxppc-dev, linux-riscv, linux-s390, linux-sh,
	sparclinux, linux-block, linux-scsi, linux-mtd,
	linux-trace-kernel, linux-pci, linux-kbuild
In-Reply-To: <3956e2a4-c545-1212-e95f-3cf61a60d6a4@gmail.com>

Hi Michael,

On Fri, Aug 25, 2023 at 3:31 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> On 25/08/23 13:12, Martin K. Petersen wrote:
> > [11/17] scsi: gvp11: remove unused gvp11_setup() function
> >          https://git.kernel.org/mkp/scsi/c/bfaa4a0ce1bb
>
> I somehow missed that one ...
>
> The gvp11_setup() function was probably a relic from the times before
> module parameters.
>
> Since gvp11_xfer_mask appears to be required for some Amiga systems to
> set the DMA mask, I'd best send a patch to add such a module parameter ...
>
> Do you know any details around the use of DMA masks for Amiga WD33C93
> drivers, Geert?

Doh, it's been a while, and I never had an affected system.
Probably it's needed on A2000 with an accelerator card and GVP II SCSI,
to prevent DMA to RAM banks that do not support fast DMA cycles.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v3 1/1] tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols
From: Masami Hiramatsu @ 2023-08-25  2:46 UTC (permalink / raw)
  To: Francis Laniel
  Cc: linux-kernel, Masami Hiramatsu, linux-trace-kernel,
	Steven Rostedt
In-Reply-To: <20230824160859.66113-2-flaniel@linux.microsoft.com>

On Thu, 24 Aug 2023 18:08:59 +0200
Francis Laniel <flaniel@linux.microsoft.com> wrote:

> 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.
> 
> Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
> Link: https://lore.kernel.org/lkml/20230819101105.b0c104ae4494a7d1f2eea742@kernel.org/

Looks good to me!

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

Thank you!

> ---
>  kernel/trace/trace_kprobe.c | 61 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 23dba01831f7..2f393739e8cf 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -705,6 +705,25 @@ static struct notifier_block trace_kprobe_module_nb = {
>  	.priority = 1	/* Invoked after kprobe module callback */
>  };
>  
> +static int count_symbols(void *data, unsigned long unused)
> +{
> +	unsigned int *count = data;
> +
> +	(*count)++;
> +
> +	return 0;
> +}
> +
> +static unsigned int number_of_same_symbols(char *func_name)
> +{
> +	unsigned int count;
> +
> +	count = 0;
> +	kallsyms_on_each_match_symbol(count_symbols, func_name, &count);
> +
> +	return count;
> +}
> +
>  static int __trace_kprobe_create(int argc, const char *argv[])
>  {
>  	/*
> @@ -836,6 +855,29 @@ static int __trace_kprobe_create(int argc, const char *argv[])
>  		}
>  	}
>  
> +	if (symbol) {
> +		unsigned int count;
> +
> +		count = number_of_same_symbols(symbol);
> +		if (count > 1) {
> +			/*
> +			 * Users should use ADDR to remove the ambiguity of
> +			 * using KSYM only.
> +			 */
> +			ret = -EADDRNOTAVAIL;
> +
> +			goto error;
> +		} else if (count == 0) {
> +			/*
> +			 * We can return ENOENT earlier than when register the
> +			 * kprobe.
> +			 */
> +			ret = -ENOENT;
> +
> +			goto error;
> +		}
> +	}
> +
>  	trace_probe_log_set_index(0);
>  	if (event) {
>  		ret = traceprobe_parse_event_name(&event, &group, gbuf,
> @@ -1699,6 +1741,7 @@ static int unregister_kprobe_event(struct trace_kprobe *tk)
>  }
>  
>  #ifdef CONFIG_PERF_EVENTS
> +
>  /* create a trace_kprobe, but don't add it to global lists */
>  struct trace_event_call *
>  create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
> @@ -1709,6 +1752,24 @@ create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
>  	int ret;
>  	char *event;
>  
> +	if (func) {
> +		unsigned int count;
> +
> +		count = number_of_same_symbols(func);
> +		if (count > 1)
> +			/*
> +			 * Users should use addr to remove the ambiguity of
> +			 * using func only.
> +			 */
> +			return ERR_PTR(-EADDRNOTAVAIL);
> +		else if (count == 0)
> +			/*
> +			 * We can return ENOENT earlier than when register the
> +			 * kprobe.
> +			 */
> +			return ERR_PTR(-ENOENT);
> +	}
> +
>  	/*
>  	 * local trace_kprobes are not added to dyn_event, so they are never
>  	 * searched in find_trace_kprobe(). Therefore, there is no concern of
> -- 
> 2.34.1
> 


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

^ permalink raw reply

* Re: (subset) [PATCH 00/17] -Wmissing-prototype warning fixes
From: Michael Schmitz @ 2023-08-25  1:30 UTC (permalink / raw)
  To: Martin K. Petersen, Andrew Morton, linux-kernel, Arnd Bergmann
  Cc: Arnd Bergmann, Matt Turner, Vineet Gupta, Russell King,
	Catalin Marinas, Will Deacon, Guo Ren, Brian Cain, Huacai Chen,
	WANG Xuerui, Geert Uytterhoeven, Michal Simek,
	Thomas Bogendoerfer, Dinh Nguyen, Jonas Bonn, Stefan Kristiansson,
	Stafford Horne, James E.J. Bottomley, Helge Deller,
	Michael Ellerman, Christophe Leroy, Palmer Dabbelt,
	Heiko Carstens, John Paul Adrian Glaubitz, x86, Borislav Petkov,
	Max Filippov, Jens Axboe, Sudip Mukherjee, Richard Weinberger,
	Bjorn Helgaas, Masahiro Yamada, Nathan Chancellor,
	Nick Desaulniers, Guenter Roeck, Stephen Rothwell, linux-next,
	linux-alpha, linux-snps-arc, linux-arm-kernel, linux-csky,
	linux-hexagon, linux-ia64, loongarch, linux-m68k, linux-mips,
	linux-openrisc, linux-parisc, linuxppc-dev, linux-riscv,
	linux-s390, linux-sh, sparclinux, linux-block, linux-scsi,
	linux-mtd, linux-trace-kernel, linux-pci, linux-kbuild
In-Reply-To: <169292577153.789945.11297239773543112051.b4-ty@oracle.com>

Hi Martin, Arnd,

On 25/08/23 13:12, Martin K. Petersen wrote:
> On Thu, 10 Aug 2023 16:19:18 +0200, Arnd Bergmann wrote:
>
>> Most of the patches I sent so far for the -Wmissing-prototype warnings
>> have made it into linux-next now. There are a few that I'm resending
>> now as nobody has picked them up, and then a number of fixes that I
>> found while test-building across all architectures rather than just the
>> ones I usually test.
>>
>> The first 15 patches in this series should be uncontroversial, so
>> I expect that either a subsystem maintainer or Andrew Morton can
>> apply these directly.
>>
>> [...]
> Applied to 6.6/scsi-queue, thanks!
>
> [07/17] scsi: qlogicpti: mark qlogicpti_info() static
>          https://git.kernel.org/mkp/scsi/c/71cc486335c4
> [11/17] scsi: gvp11: remove unused gvp11_setup() function
>          https://git.kernel.org/mkp/scsi/c/bfaa4a0ce1bb

I somehow missed that one ...

The gvp11_setup() function was probably a relic from the times before 
module parameters.

Since gvp11_xfer_mask appears to be required for some Amiga systems to 
set the DMA mask, I'd best send a patch to add such a module parameter ...

Do you know any details around the use of DMA masks for Amiga WD33C93 
drivers, Geert?

Cheers,

     Michael


>

^ permalink raw reply

* Re: (subset) [PATCH 00/17] -Wmissing-prototype warning fixes
From: Martin K. Petersen @ 2023-08-25  1:12 UTC (permalink / raw)
  To: Andrew Morton, linux-kernel, Arnd Bergmann
  Cc: Martin K . Petersen, Arnd Bergmann, Matt Turner, Vineet Gupta,
	Russell King, Catalin Marinas, Will Deacon, Guo Ren, Brian Cain,
	Huacai Chen, WANG Xuerui, Geert Uytterhoeven, Michal Simek,
	Thomas Bogendoerfer, Dinh Nguyen, Jonas Bonn, Stefan Kristiansson,
	Stafford Horne, James E.J. Bottomley, Helge Deller,
	Michael Ellerman, Christophe Leroy, Palmer Dabbelt,
	Heiko Carstens, John Paul Adrian Glaubitz, x86, Borislav Petkov,
	Max Filippov, Jens Axboe, Sudip Mukherjee, Richard Weinberger,
	Bjorn Helgaas, Masahiro Yamada, Nathan Chancellor,
	Nick Desaulniers, Guenter Roeck, Stephen Rothwell, linux-next,
	linux-alpha, linux-snps-arc, linux-arm-kernel, linux-csky,
	linux-hexagon, linux-ia64, loongarch, linux-m68k, linux-mips,
	linux-openrisc, linux-parisc, linuxppc-dev, linux-riscv,
	linux-s390, linux-sh, sparclinux, linux-block, linux-scsi,
	linux-mtd, linux-trace-kernel, linux-pci, linux-kbuild
In-Reply-To: <20230810141947.1236730-1-arnd@kernel.org>

On Thu, 10 Aug 2023 16:19:18 +0200, Arnd Bergmann wrote:

> Most of the patches I sent so far for the -Wmissing-prototype warnings
> have made it into linux-next now. There are a few that I'm resending
> now as nobody has picked them up, and then a number of fixes that I
> found while test-building across all architectures rather than just the
> ones I usually test.
> 
> The first 15 patches in this series should be uncontroversial, so
> I expect that either a subsystem maintainer or Andrew Morton can
> apply these directly.
> 
> [...]

Applied to 6.6/scsi-queue, thanks!

[07/17] scsi: qlogicpti: mark qlogicpti_info() static
        https://git.kernel.org/mkp/scsi/c/71cc486335c4
[11/17] scsi: gvp11: remove unused gvp11_setup() function
        https://git.kernel.org/mkp/scsi/c/bfaa4a0ce1bb

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply

* Re: [RFC PATCH v2 1/1] tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols
From: Francis Laniel @ 2023-08-24 16:09 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: linux-kernel, linux-trace-kernel, Steven Rostedt
In-Reply-To: <20230824234721.1b481cd5d0b8bbc43a24d9a6@kernel.org>

Le jeudi 24 août 2023, 16:47:21 CEST Masami Hiramatsu a écrit :
> On Thu, 24 Aug 2023 16:31:13 +0200
> 
> Francis Laniel <flaniel@linux.microsoft.com> wrote:
> > Hi.
> > 
> > Le jeudi 24 août 2023, 15:02:27 CEST Masami Hiramatsu a écrit :
> > > On Thu, 24 Aug 2023 12:37:34 +0200
> > > 
> > > Francis Laniel <flaniel@linux.microsoft.com> wrote:
> > > > 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
> > > > call, because the effectively called one has no kprobes.
> > > > 
> > > > So, this commit returns EADDRNOTAVAIL when func matches several
> > > > symbols.
> > > > This way, user needs to use addr to remove the ambiguity.
> > > 
> > > Thanks for update the patch. I have some comments there.
> > > 
> > > > Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > > Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
> > > > Link:
> > > > https://lore.kernel.org/lkml/20230819101105.b0c104ae4494a7d1f2eea742@k
> > > > ern
> > > > el.org/ ---
> > > > 
> > > >  kernel/trace/trace_kprobe.c | 42
> > > >  +++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 42 insertions(+)
> > > > 
> > > > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > > > index 23dba01831f7..0c8dd6ba650b 100644
> > > > --- a/kernel/trace/trace_kprobe.c
> > > > +++ b/kernel/trace/trace_kprobe.c
> > > > @@ -705,6 +705,25 @@ static struct notifier_block
> > > > trace_kprobe_module_nb =
> > > > {>
> > > > 
> > > >  	.priority = 1	/* Invoked after kprobe module callback */
> > > >  
> > > >  };
> > > > 
> > > > +static int count_symbols(void *data, unsigned long unused)
> > > > +{
> > > > +	unsigned int *count = data;
> > > > +
> > > > +	(*count)++;
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static unsigned int func_name_several_symbols(char *func_name)
> > > 
> > > If this returns boolean, please use 'bool' for return type.
> > > Also, I think 'func_name_is_unique()' is more natural.
> > 
> > This name sounds better but it means it will check count == 1.
> > I am fine with it, but please see my below comment.
> > 
> > > > +{
> > > > +	unsigned int count;
> > > > +
> > > > +	count = 0;
> > > > +	kallsyms_on_each_match_symbol(count_symbols, func_name, &count);
> > > > +
> > > > +	return count > 1;
> > > > +}
> > > > +
> > > > 
> > > >  static int __trace_kprobe_create(int argc, const char *argv[])
> > > >  {
> > > >  
> > > >  	/*
> > > > 
> > > > @@ -836,6 +855,18 @@ static int __trace_kprobe_create(int argc, const
> > > > char
> > > > *argv[])>
> > > > 
> > > >  		}
> > > >  	
> > > >  	}
> > > > 
> > > > +	/*
> > > > +	 * If user specifies KSYM, we check it does not correspond to
> > > > several
> > > > +	 * symbols.
> > > > +	 * If this is the case, we return EADDRNOTAVAIL to indicate the user
> > > > +	 * he/she should use ADDR rather than KSYM to remove the ambiguity.
> > > > +	 */
> > > > +	if (symbol && func_name_several_symbols(symbol)) {
> > > 
> > > Then, here  will be
> > > 
> > > 	if (symbol && !func_name_is_unique(symbol)) {
> > 
> > I am fine with the above, but it means if users gives an unknown symbol,
> > we
> > will return EADDRNOTAVAIL instead of currently returning ENOENT.
> > Is it OK?
> 
> Ah, good catch! Hm, then what about 'int number_of_same_symbols()' ?
> 
> 
> if (symbol) {
> 	num = number_of_same_symbols(symbol);
> 	if (num > 1)
> 		return -EADDRNOTAVAIL;
> 	else if (num == 0)
> 		return -ENOENT;
> }

Done in the v3 :D!

> Thank you,
> 
> > > > +		ret = -EADDRNOTAVAIL;
> > > > +
> > > > +		goto error;
> > > > +	}
> > > > +
> > > > 
> > > >  	trace_probe_log_set_index(0);
> > > >  	if (event) {
> > > >  	
> > > >  		ret = traceprobe_parse_event_name(&event, &group, gbuf,
> > > > 
> > > > @@ -1699,6 +1730,7 @@ static int unregister_kprobe_event(struct
> > > > trace_kprobe *tk)>
> > > > 
> > > >  }
> > > >  
> > > >  #ifdef CONFIG_PERF_EVENTS
> > > > 
> > > > +
> > > > 
> > > >  /* create a trace_kprobe, but don't add it to global lists */
> > > >  struct trace_event_call *
> > > >  create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
> > > > 
> > > > @@ -1709,6 +1741,16 @@ create_local_trace_kprobe(char *func, void
> > > > *addr,
> > > > unsigned long offs,>
> > > > 
> > > >  	int ret;
> > > >  	char *event;
> > > > 
> > > > +	/*
> > > > +	 * If user specifies func, we check that function name does not
> > > > +	 * correspond to several symbols.
> > > > +	 * If this is the case, we return EADDRNOTAVAIL to indicate the user
> > > > +	 * he/she should use addr and offs rather than func to remove the
> > > > +	 * ambiguity.
> > > > +	 */
> > > > +	if (func && func_name_several_symbols(func))
> > > 
> > > Ditto.
> > > 
> > > Thanks!
> > > 
> > > > +		return ERR_PTR(-EADDRNOTAVAIL);
> > > > +
> > > > 
> > > >  	/*
> > > >  	
> > > >  	 * local trace_kprobes are not added to dyn_event, so they are never
> > > >  	 * searched in find_trace_kprobe(). Therefore, there is no concern
> > > >  	 of
> > 
> > Best regards.





^ permalink raw reply

* [PATCH v3 0/1] Return EADDRNOTAVAIL when func matches several symbols during kprobe creation
From: Francis Laniel @ 2023-08-24 16:08 UTC (permalink / raw)
  To: linux-kernel; +Cc: Masami Hiramatsu, linux-trace-kernel, Francis Laniel

Hi.


In the kernel source code, it exists different functions which share the same
name but which have, of course, different addresses as they can be defined in
different modules:
# Kernel was compiled with CONFIG_NTFS_FS and CONFIG_NTFS3_FS as built-in.
root@vm-amd64:~# grep ntfs_file_write_iter /proc/kallsyms
ffffffff814ce3c0 t __pfx_ntfs_file_write_iter
ffffffff814ce3d0 t ntfs_file_write_iter
ffffffff814fc8a0 t __pfx_ntfs_file_write_iter
ffffffff814fc8b0 t ntfs_file_write_iter
This can be source of troubles when you create a PMU kprobe for such a function,
as it will only install one for the first address (e.g. 0xffffffff814ce3d0 in
the above).
This could lead to some troubles were BPF based tools does not report any event
because the second function is not called:
root@vm-amd64:/mnt# mount | grep /mnt
/foo.img on /mnt type ntfs3 (rw,relatime,uid=0,gid=0,iocharset=utf8)
# ig is a tool which installs a PMU kprobe on ntfs_file_write_iter().
root@vm-amd64:/mnt# ig trace fsslower -m 0 -f ntfs3 --host &> /tmp/foo &
[1] 207
root@vm-amd64:/mnt# dd if=./foo of=./bar count=3
3+0 records in
3+0 records out
1536 bytes (1.5 kB, 1.5 KiB) copied, 0.00543323 s, 283 kB/s
root@vm-amd64:/mnt# fg
ig trace fsslower -m 0 -f ntfs3 --host &> /tmp/foo
^Croot@vm-amd64:/mnt# more /tmp/foo
RUNTIME.CONTAINERNAME          RUNTIME.CONTAIN… PID              COMM
  T      BYTES     OFFSET        LAT FILE
                                                214              dd
  R        512          0        766 foo
                                                214              dd
  R        512        512          9 foo
                                                214              dd
As you can see in the above, only read events are reported and no write because
the kprobe is installed for the old ntfs_file_write_iter() and not the ntfs3
one.
The same behavior occurs with sysfs kprobe:
root@vm-amd64:/# echo 'p:probe/ntfs_file_write_iter ntfs_file_write_iter' > /sys/kernel/tracing/kprobe_events
root@vm-amd64:/# cat /sys/kernel/tracing/kprobe_events
p:probe/ntfs_file_write_iter ntfs_file_write_iter
root@vm-amd64:/# mount | grep /mnt
/foo.img on /mnt type ntfs3 (rw,relatime,uid=0,gid=0,iocharset=utf8)
root@vm-amd64:/# perf record -e probe:ntfs_file_write_iter &
[1] 210
root@vm-amd64:/# cd /mnt/
root@vm-amd64:/mnt# dd if=./foo of=./bar count=3
3+0 records in
3+0 records out
1536 bytes (1.5 kB, 1.5 KiB) copied, 0.00234793 s, 654 kB/s
root@vm-amd64:/mnt# cd -
/
root@vm-amd64:/# fg
perf record -e probe:ntfs_file_write_iter
^C[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.056 MB perf.data ]

root@vm-amd64:/# perf report
Error:
The perf.data data has no samples!
# To display the perf.data header info, please use --header/--header-only optio>
#

In this contribution, I modified the functions creating sysfs and PMU kprobes to
test if the function name given as argument matches several symbols.
In this case, these functions return EADDRNOTAVAIL to indicate the user to use
addr and offs to remove this ambiguity.
So, when the above BPF tool is run, the following error message is printed:
root@vm-amd64:~# ig trace fsslower -m 0 -f ntfs3 --host &> /tmp/foo &
[1] 228
root@vm-amd64:~# more /tmp/foo
RUNTIME.CONTAINERNAME          RUNTIME.CONTAIN… PID              COMM
  T      BYTES     OFFSET        LAT FILE
Error: running gadget: running gadget: installing tracer: attaching kprobe: crea
ting perf_kprobe PMU (arch-specific fallback for "ntfs_file_write_iter"): token
ntfs_file_write_iter: opening perf event: cannot assign requested address
And the same with sysfs kprobe:
root@vm-amd64:/# echo 'p:probe/ntfs_file_write_iter ntfs_file_write_iter' > /sys/kernel/tracing/kprobe_events
-bash: echo: write error: Cannot assign requested address
Note that, this does not influence perf as it installs kprobes as offset on
_text:
root@vm-amd64:/# perf probe --add ntfs_file_write_iter
Added new events:
  probe:ntfs_file_write_iter (on ntfs_file_write_iter)
  probe:ntfs_file_write_iter (on ntfs_file_write_iter)
...
root@vm-amd64:/# cat /sys/kernel/tracing/kprobe_events
p:probe/ntfs_file_write_iter _text+5039088
p:probe/ntfs_file_write_iter _text+5228752

Note that, this contribution is the conclusion of a previous RFC which intended
to install a PMU kprobe for all matching symbols [1, 2].

If you see any way to improve this contribution, particularly if you have an
idea to add tests or documentation for this behavior, please share your
feedback.

Changes since:
 v1:
  * Use EADDRNOTAVAIL instead of adding a new error code.
  * Correct also this behavior for sysfs kprobe.
 v2:
  * Count the number of symbols corresponding to function name and return
  EADDRNOTAVAIL if higher than 1.
  * Return ENOENT if above count is 0, as it would be returned later by while
  registering the kprobe.

Francis Laniel (1):
  tracing/kprobes: Return EADDRNOTAVAIL when func matches several
    symbols

 kernel/trace/trace_kprobe.c | 61 +++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)


Best regards and thank you in advance.
---
[1]: https://lore.kernel.org/lkml/20230816163517.112518-1-flaniel@linux.microsoft.com/
[2]: https://lore.kernel.org/lkml/20230819101105.b0c104ae4494a7d1f2eea742@kernel.org/
--
2.34.1


^ permalink raw reply

* [PATCH v3 1/1] tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols
From: Francis Laniel @ 2023-08-24 16:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, linux-trace-kernel, Francis Laniel,
	Steven Rostedt
In-Reply-To: <20230824160859.66113-1-flaniel@linux.microsoft.com>

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.

Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
Link: https://lore.kernel.org/lkml/20230819101105.b0c104ae4494a7d1f2eea742@kernel.org/
---
 kernel/trace/trace_kprobe.c | 61 +++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 23dba01831f7..2f393739e8cf 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -705,6 +705,25 @@ static struct notifier_block trace_kprobe_module_nb = {
 	.priority = 1	/* Invoked after kprobe module callback */
 };
 
+static int count_symbols(void *data, unsigned long unused)
+{
+	unsigned int *count = data;
+
+	(*count)++;
+
+	return 0;
+}
+
+static unsigned int number_of_same_symbols(char *func_name)
+{
+	unsigned int count;
+
+	count = 0;
+	kallsyms_on_each_match_symbol(count_symbols, func_name, &count);
+
+	return count;
+}
+
 static int __trace_kprobe_create(int argc, const char *argv[])
 {
 	/*
@@ -836,6 +855,29 @@ static int __trace_kprobe_create(int argc, const char *argv[])
 		}
 	}
 
+	if (symbol) {
+		unsigned int count;
+
+		count = number_of_same_symbols(symbol);
+		if (count > 1) {
+			/*
+			 * Users should use ADDR to remove the ambiguity of
+			 * using KSYM only.
+			 */
+			ret = -EADDRNOTAVAIL;
+
+			goto error;
+		} else if (count == 0) {
+			/*
+			 * We can return ENOENT earlier than when register the
+			 * kprobe.
+			 */
+			ret = -ENOENT;
+
+			goto error;
+		}
+	}
+
 	trace_probe_log_set_index(0);
 	if (event) {
 		ret = traceprobe_parse_event_name(&event, &group, gbuf,
@@ -1699,6 +1741,7 @@ static int unregister_kprobe_event(struct trace_kprobe *tk)
 }
 
 #ifdef CONFIG_PERF_EVENTS
+
 /* create a trace_kprobe, but don't add it to global lists */
 struct trace_event_call *
 create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
@@ -1709,6 +1752,24 @@ create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
 	int ret;
 	char *event;
 
+	if (func) {
+		unsigned int count;
+
+		count = number_of_same_symbols(func);
+		if (count > 1)
+			/*
+			 * Users should use addr to remove the ambiguity of
+			 * using func only.
+			 */
+			return ERR_PTR(-EADDRNOTAVAIL);
+		else if (count == 0)
+			/*
+			 * We can return ENOENT earlier than when register the
+			 * kprobe.
+			 */
+			return ERR_PTR(-ENOENT);
+	}
+
 	/*
 	 * local trace_kprobes are not added to dyn_event, so they are never
 	 * searched in find_trace_kprobe(). Therefore, there is no concern of
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v2] scripts/link-vmlinux.sh: Add alias to duplicate symbols for kallsyms
From: Francis Laniel @ 2023-08-24 15:35 UTC (permalink / raw)
  To: Steven Rostedt, Alexander Lobakin, Nick Alcock,
	Alessandro Carminati
  Cc: Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, Masami Hiramatsu, Daniel Bristot de Oliveira,
	Viktor Malik, Kris Van Hees, Luis Chamberlain, eugene.loh,
	Josh Poimboeuf, linux-kernel, linux-kbuild, live-patching,
	linux-trace-kernel
In-Reply-To: <CAPp5cGRBW6WDm9ku3nsr0x=rhEOGAUx==BctCo+DJ+498Tz2Sw@mail.gmail.com>

Hi.

Le vendredi 21 juillet 2023, 14:40:54 CEST Alessandro Carminati a écrit :
> Hello,
> 
> I apologize for being noisy today.
> 
> In an effort to be collaborative, I would like to share my thoughts on why I
> see duplicate symbols in fs/binfmt_elf.c.
> 
> Il giorno ven 21 lug 2023 alle ore 11:22 Alessandro Carminati
> 
> <alessandro.carminati@gmail.com> ha scritto:
> > Hello Steven, Alexander, and Nick,
> > 
> > Since now I realize that a lot of people are working on my very same
> > argument, I'm writing you this mail just with the intent of not leaving
> > my statements floating in the air, and help the cause by sharing my
> > experience, as little it is, to solve this problem.
> > 
> > The addr2line strategy, I tried to implement, allows me to achieve the
> > level of accuracy that Luis mentioned in his comment about live-patching
> > suggestions. The addr2line-based approach is feasible, however, to make
> > this approach operable, I had to ensure that vmlinux contains the
> > necessary debug information.
> > Let me clarify.
> > During the kernel build, I do need the vmlinux to contain debug
> > information. The approach I pursued parses the nm output and, when it
> > detects a duplicate symbol, it needs to query vmlinux using addr2line to
> > resolve the address where the symbol is reported to be.
> > The whole process takes place during the building phase.
> > I've completed the work to integrate the new alias within the compilation
> > phase, and I wanted to share with you how it looks now.
> > 
> > ```
> > ~ # uname -a
> > Linux (none) 6.4.0 #1 SMP PREEMPT Thu Jul 20 10:05:36 UTC 2023 aarch64
> > GNU/Linux ~ # cat /proc/kallsyms | grep gic_mask_irq
> > ffffacf2eb64dae4 t gic_mask_irq
> > ffffacf2eb64dae4 t gic_mask_irq@_drivers_irqchip_irq-gic_c_167
> > ffffacf2eb650960 t gic_mask_irq
> > ffffacf2eb650960 t gic_mask_irq@_drivers_irqchip_irq-gic-v3_c_404
> > ~ #
> > ```
> > 
> > Despite this appearing to have a sufficient level of accuracy, when I
> > tested it, I realized something I really did not expect. Multiple
> > instances of the same function exist, and when I say "same function," I
> > don't just mean the name, but also the function's body. I apologize for
> > stating something that may sound obvious to you, but I found this really
> > unexpected.
> > 
> > ```
> > ~ # uname -a
> > Linux (none) 6.4.0 #1 SMP PREEMPT Thu Jul 20 10:05:36 UTC 2023 aarch64
> > GNU/Linux ~ # cat /proc/kallsyms | grep -E " [ttT] " | cut -d" "
> > -f3|sort| uniq -d| grep @_
> > BIT_initDStream@_lib_zstd_common_bitstream_h_259
> > __dev_hold@_include_linux_netdevice_h_4059
> > __dev_put@_include_linux_netdevice_h_4048
> > __flush_tlb_range@_arch_arm64_include_asm_tlbflush_h_291
> > __flush_tlb_range_constprop_0@_arch_arm64_include_asm_tlbflush_h_291
> > __kern_my_cpu_offset@_arch_arm64_include_asm_percpu_h_40
> > __kvm_nvhe_$x@_arch_arm64_kvm_hyp_include_nvhe_memory_h_22
> > __kvm_nvhe___kvm_skip_instr@_arch_arm64_kvm_hyp_include_hyp_adjust_pc_h_34
> > __kvm_nvhe_hyp_page_count@_arch_arm64_kvm_hyp_include_nvhe_memory_h_47
> > __kvm_nvhe_hyp_phys_to_virt@_arch_arm64_kvm_hyp_include_nvhe_memory_h_22
> > __kvm_nvhe_hyp_virt_to_phys@_arch_arm64_kvm_hyp_include_nvhe_memory_h_27
> > __kvm_skip_instr@_arch_arm64_kvm_hyp_include_hyp_adjust_pc_h_34
> > __nodes_weight_constprop_0@_include_linux_nodemask_h_239
> > __pi_$x@_scripts_dtc_libfdt_libfdt_h_145
> > __pi_fdt32_ld@_scripts_dtc_libfdt_libfdt_h_145
> > __preempt_count_dec_and_test@_arch_arm64_include_asm_current_h_19
> > acpi_dev_filter_resource_type_cb@_include_linux_acpi_h_520
> > add_quirk@_drivers_mmc_core_card_h_158
> > bpf_enable_instrumentation@_include_linux_bpf_h_1937
> > btf_id_cmp_func@_include_linux_btf_h_469
> > copy_from_sockptr_offset_constprop_0@_include_linux_sockptr_h_44
> > cpucap_multi_entry_cap_matches@_arch_arm64_include_asm_cpufeature_h_395
> > cpufreq_register_em_with_opp@_include_linux_cpufreq_h_1228
> > cpumask_weight@_include_linux_cpumask_h_691
> > cpumask_weight_constprop_0@_include_linux_cpumask_h_690
> > css_put@_include_linux_cgroup_refcnt_h_77
> > dma_cookie_status@_drivers_dma_dmaengine_h_74
> > dst_discard@_include_net_dst_h_392
> > dst_output@_include_net_dst_h_457
> > elf_core_dump@_fs_binfmt_elf_c_2027
> > fixup_use_fwh_lock@_drivers_mtd_chips_fwh_lock_h_102
> > flush_tlb_mm@_arch_arm64_include_asm_tlbflush_h_250
> > fwh_lock_varsize@_drivers_mtd_chips_fwh_lock_h_81
> > fwh_unlock_varsize@_drivers_mtd_chips_fwh_lock_h_92
> > fwh_xxlock_oneblock@_drivers_mtd_chips_fwh_lock_h_31
> > get_net_ns@_include_net_net_namespace_h_231
> > inline_map_read_isra_0@_include_linux_mtd_map_h_393
> > inline_map_write_part_0@_include_linux_mtd_map_h_426
> > io_run_task_work@_io_uring_io_uring_h_282
> > ipv6_portaddr_hash_isra_0@_include_net_ipv6_h_732
> > irq_find_host@_include_linux_irqdomain_h_322
> > jhash@_include_linux_jhash_h_76
> > jhash_constprop_0@_include_linux_jhash_h_76
> > ktime_get_boottime@_include_linux_timekeeping_h_94
> > ktime_get_real@_include_linux_timekeeping_h_78
> > load_elf_binary@_fs_binfmt_elf_c_824
> > load_elf_phdrs@_fs_binfmt_elf_c_468
> > may_use_simd@_arch_arm64_include_asm_alternative-macros_h_232
> > netif_tx_disable@_include_linux_netdevice_h_4486
> > of_parse_phandle@_include_linux_of_h_946
> > of_parse_phandle_constprop_0@_include_linux_of_h_943
> > padzero@_fs_binfmt_elf_c_142
> > percpu_down_read_constprop_0@_include_linux_percpu-rwsem_h_47
> > percpu_ref_get_many@_include_linux_percpu-refcount_h_199
> > percpu_ref_get_many_constprop_0@_include_linux_percpu-refcount_h_198
> > percpu_ref_put_many_constprop_0@_include_linux_percpu-refcount_h_326
> > percpu_up_read@_include_linux_percpu-rwsem_h_98
> > percpu_up_read_constprop_0@_include_linux_percpu-rwsem_h_97
> > pinconf_generic_dt_node_to_map_all@_include_linux_pinctrl_pinconf-generic_
> > h_223
> > pinconf_generic_dt_node_to_map_group@_include_linux_pinctrl_pinconf-gener
> > ic_h_207
> > pinconf_generic_dt_node_to_map_pin@_include_linux_pinctrl_pinconf-generic
> > _h_215
> > platform_device_register_resndata_constprop_0@_include_linux_platform_dev
> > ice_h_125
> > ptrauth_keys_install_user@_arch_arm64_include_asm_alternative-macros_h_23
> > 2 readl@_arch_arm64_include_asm_io_h_75
> > reqsk_put@_include_net_request_sock_h_133
> > rht_key_get_hash_constprop_0_isra_0@_include_linux_rhashtable_h_133
> > rht_key_get_hash_isra_0@_include_linux_rhashtable_h_125
> > role_show@_include_linux_device_h_763
> > sdhci_and_cqhci_reset@_drivers_mmc_host_sdhci-cqhci_h_16
> > set_active_memcg_part_0@_include_linux_sched_mm_h_410
> > set_brk@_fs_binfmt_elf_c_114
> > set_is_seen@_arch_arm64_include_asm_current_h_19
> > set_lookup@_arch_arm64_include_asm_current_h_19
> > sha1_base_init@_include_crypto_sha1_base_h_22
> > sha224_base_init@_include_crypto_sha256_base_h_23
> > sha256_base_init@_include_crypto_sha256_base_h_31
> > spi_sync_transfer_constprop_0@_include_linux_spi_spi_h_1339
> > spi_write@_include_linux_spi_spi_h_1363
> > tlb_flush@_arch_arm64_include_asm_tlb_h_54
> > trace_xhci_dbg_quirks@_drivers_usb_host_xhci-trace_h_48
> > udp_lib_close@_include_net_udp_h_197
> > udp_lib_hash@_include_net_udp_h_189
> > virtio_net_hdr_to_skb_constprop_0@_include_linux_virtio_net_h_48
> > writenote@_fs_binfmt_elf_c_1479
> > zero_user_segments@_include_linux_highmem_h_271
> > zero_user_segments_constprop_0@_include_linux_highmem_h_268
> > ```
> > 
> > The previous evidence demonstrates that adding a tag with a file and line
> > number does not make a symbol unique.
> > 
> > Here, I inspect the function body to verify if the functions are indeed
> > the same.
> > 
> > ```
> > Welcome to Buildroot
> > buildroot login: root
> > ~ # cat /proc/kallsyms | grep set_brk
> > ffffa8a9d1cf1d2c t set_brk
> > ffffa8a9d1cf1d2c t set_brk@_fs_binfmt_elf_c_114
> > ffffa8a9d1cf4454 t set_brk
> > ffffa8a9d1cf4454 t set_brk@_fs_binfmt_elf_c_114
> > ~ # QEMU: Terminated
> > $ cat System.map | grep set_brk
> > ffff8000082f1d2c t set_brk
> > ffff8000082f4454 t set_brk
> > ~ $ r2 vmlinux
> > Warning: run r2 with -e bin.cache=true to fix relocations in disassembly
> > 
> >  -- Reduce the delta where flag resolving by address is used with
> >  cfg.delta
> > 
> > [0xffff800008000000]> s 0xffff8000082f1d2c
> > [0xffff8000082f1d2c]> aa
> > [x] Analyze all flags starting with sym. and entry0 (aa)
> > [x] Analyze all functions arguments/locals
> > [0xffff8000082f1d2c]> pdf
> > 
> >             ; CALL XREFS from sym.load_elf_binary @ 0xffff8000082f352c(x),
> >             0xffff8000082f38d4(x), 0xffff8000082f3a68(x)> 
> > ┌ 112: sym.set_brk (int64_t arg1, int64_t arg2, int64_t arg3);
> > │           ; arg int64_t arg1 @ x0
> > │           ; arg int64_t arg2 @ x1
> > │           ; arg int64_t arg3 @ x2
> > │           ; var int64_t var_10h @ sp+0x10
> > │           0xffff8000082f1d2c      3f2303d5       paciasp             ;
> > binfmt_elf.c:114 │           0xffff8000082f1d30      fd7bbea9       stp
> > x29, x30, [sp, -0x20]! │           0xffff8000082f1d34      00fc3f91      
> > add x0, x0, 0xfff   ; binfmt_elf.c:115 ; arg1 │          
> > 0xffff8000082f1d38      fd030091       mov x29, sp         ;
> > binfmt_elf.c:114 │           0xffff8000082f1d3c      21fc3f91       add
> > x1, x1, 0xfff   ; binfmt_elf.c:116 ; arg2 │           0xffff8000082f1d40 
> >     00cc7492       and x0, x0, 0xfffffffffffff000 ; binfmt_elf.c:114 ;
> > arg1 │           0xffff8000082f1d44      f30b00f9       str x19, [sp,
> > 0x10] ; binfmt_elf.c:116 │           0xffff8000082f1d48      33cc7492    
> >   and x19, x1, 0xfffffffffffff000 ; arg2 │           0xffff8000082f1d4c  
> >    1f0013eb       cmp x0, x19         ; binfmt_elf.c:117 ; arg1 │      
> > ┌─< 0xffff8000082f1d50      63010054       b.lo 0xffff8000082f1d7c │     
> > ┌──> 0xffff8000082f1d54      014138d5       mrs x1, sp_el0      ;
> > binfmt_elf.c:128 │      ╎│   0xffff8000082f1d58      22fc41f9       ldr
> > x2, [x1, 0x3f8] ; current.h:21 ; 0xd9 ; 217 │      ╎│  
> > 0xffff8000082f1d5c      00008052       mov w0, 0           ;
> > binfmt_elf.c:129 │      ╎│   0xffff8000082f1d60      539400f9       str
> > x19, [x2, 0x128] ; binfmt_elf.c:128 │      ╎│   0xffff8000082f1d64     
> > 21fc41f9       ldr x1, [x1, 0x3f8] ; current.h:17 ; 0xd9 ; 217 │      ╎│ 
> >  0xffff8000082f1d68      339000f9       str x19, [x1, 0x120] ;
> > binfmt_elf.c:128 │      ╎│   0xffff8000082f1d6c      f30b40f9       ldr
> > x19, [var_10h]  ; binfmt_elf.c:129 ; 5 │      ╎│   0xffff8000082f1d70    
> >  fd7bc2a8       ldp x29, x30, [sp], 0x20 │      ╎│   0xffff8000082f1d74  
> >    bf2303d5       autiasp
> > │      ╎│   0xffff8000082f1d78      c0035fd6       ret
> > │      ╎└─> 0xffff8000082f1d7c      42007e92       and x2, x2, 4       ;
> > binfmt_elf.c:123 ; arg3 │      ╎    0xffff8000082f1d80      610200cb     
> >  sub x1, x19, x0     ; arg1 │      ╎    0xffff8000082f1d84      cae3fc97 
> >      bl sym.vm_brk_flags │      └──< 0xffff8000082f1d88      60feff34    
> >   cbz w0, 0xffff8000082f1d54 ; binfmt_elf.c:125 │          
> > 0xffff8000082f1d8c      f30b40f9       ldr x19, [var_10h]  ;
> > binfmt_elf.c:130 ; 5 │           0xffff8000082f1d90      fd7bc2a8      
> > ldp x29, x30, [sp], 0x20 │           0xffff8000082f1d94      bf2303d5    
> >   autiasp
> > └           0xffff8000082f1d98      c0035fd6       ret
> > [0xffff8000082f1d2c]> s  0xffff8000082f4454
> > [0xffff8000082f4454]> pdf
> > 
> >             ; CALL XREFS from sym.load_elf_binary_1 @
> >             0xffff8000082f5cc0(x), 0xffff8000082f5e5c(x),
> >             0xffff8000082f6648(x)> 
> > ┌ 112: sym.set_brk_1 (int64_t arg1, int64_t arg2, int64_t arg3);
> > │           ; arg int64_t arg1 @ x0
> > │           ; arg int64_t arg2 @ x1
> > │           ; arg int64_t arg3 @ x2
> > │           ; var int64_t var_10h @ sp+0x10
> > │           0xffff8000082f4454      3f2303d5       paciasp             ;
> > binfmt_elf.c:114 │           0xffff8000082f4458      fd7bbea9       stp
> > x29, x30, [sp, -0x20]! │           0xffff8000082f445c      00fc3f91      
> > add x0, x0, 0xfff   ; binfmt_elf.c:115 ; arg1 │          
> > 0xffff8000082f4460      fd030091       mov x29, sp         ;
> > binfmt_elf.c:114 │           0xffff8000082f4464      21fc3f91       add
> > x1, x1, 0xfff   ; binfmt_elf.c:116 ; arg2 │           0xffff8000082f4468 
> >     00cc7492       and x0, x0, 0xfffffffffffff000 ; binfmt_elf.c:114 ;
> > arg1 │           0xffff8000082f446c      f30b00f9       str x19, [sp,
> > 0x10] ; binfmt_elf.c:116 │           0xffff8000082f4470      33cc7492    
> >   and x19, x1, 0xfffffffffffff000 ; arg2 │           0xffff8000082f4474  
> >    1f0013eb       cmp x0, x19         ; binfmt_elf.c:117 ; arg1 │      
> > ┌─< 0xffff8000082f4478      63010054       b.lo 0xffff8000082f44a4 │     
> > ┌──> 0xffff8000082f447c      014138d5       mrs x1, sp_el0      ;
> > binfmt_elf.c:128 │      ╎│   0xffff8000082f4480      22fc41f9       ldr
> > x2, [x1, 0x3f8] ; current.h:21 ; 0xd9 ; 217 │      ╎│  
> > 0xffff8000082f4484      00008052       mov w0, 0           ;
> > binfmt_elf.c:129 │      ╎│   0xffff8000082f4488      539400f9       str
> > x19, [x2, 0x128] ; binfmt_elf.c:128 │      ╎│   0xffff8000082f448c     
> > 21fc41f9       ldr x1, [x1, 0x3f8] ; current.h:17 ; 0xd9 ; 217 │      ╎│ 
> >  0xffff8000082f4490      339000f9       str x19, [x1, 0x120] ;
> > binfmt_elf.c:128 │      ╎│   0xffff8000082f4494      f30b40f9       ldr
> > x19, [var_10h]  ; binfmt_elf.c:129 ; 5 │      ╎│   0xffff8000082f4498    
> >  fd7bc2a8       ldp x29, x30, [sp], 0x20 │      ╎│   0xffff8000082f449c  
> >    bf2303d5       autiasp
> > │      ╎│   0xffff8000082f44a0      c0035fd6       ret
> > │      ╎└─> 0xffff8000082f44a4      42007e92       and x2, x2, 4       ;
> > binfmt_elf.c:123 ; arg3 │      ╎    0xffff8000082f44a8      610200cb     
> >  sub x1, x19, x0     ; arg1 │      ╎    0xffff8000082f44ac      00dafc97 
> >      bl sym.vm_brk_flags │      └──< 0xffff8000082f44b0      60feff34    
> >   cbz w0, 0xffff8000082f447c ; binfmt_elf.c:125 │          
> > 0xffff8000082f44b4      f30b40f9       ldr x19, [var_10h]  ;
> > binfmt_elf.c:130 ; 5 │           0xffff8000082f44b8      fd7bc2a8      
> > ldp x29, x30, [sp], 0x20 │           0xffff8000082f44bc      bf2303d5    
> >   autiasp
> > └           0xffff8000082f44c0      c0035fd6       ret
> > [0xffff8000082f4454]>
> > ```
> > 
> > While I can speculate on a reasonable explanation for why I see symbols
> > coming from header files being duplicated – when a header file is
> > included in a C file and it produces a function that the compiler does
> > not want to inline, it is replicated every time the file is included – I
> > have no convincing explanation as to why the fs/binfmt.c functions exist
> > in more than one instance.
> > 
> > ```
> > elf_core_dump@_fs_binfmt_elf_c_2027
> > load_elf_binary@_fs_binfmt_elf_c_824
> > load_elf_phdrs@_fs_binfmt_elf_c_468
> > padzero@_fs_binfmt_elf_c_142
> > set_brk@_fs_binfmt_elf_c_114
> > writenote@_fs_binfmt_elf_c_1479
> > ```
> 
> After examining the contents of built-in.a, I have come up with a new
> interpretation of what I am observing.
> According to built-in.a, System.map, and vmlinux there are two symbols named
> set_brk.
> 
> ```
> ~ $ cat System.map | grep set_brk
> ffff8000082f1d2c t set_brk
> ffff8000082f4454 t set_brk
> ~ $
> ~ $ nm -n vmlinux | grep set_brk
> ffff8000082f1d2c t set_brk
> ffff8000082f4454 t set_brk
> ~ $
> ~ $ nm -n built-in.a| grep set_brk
> 00000000000000d4 t set_brk
> 00000000000000d4 t set_brk
> ~ $
> ~ $ nm -n built-in.a | grep set_brk -B100| egrep "set_brk|\.o:$"
> fs/binfmt_elf.o:
> 00000000000000d4 t set_brk
> fs/compat_binfmt_elf.o:
> 00000000000000d4 t set_brk
> ```
> 
> These two symbols come from fs/binfmt_elf.o and fs/compat_binfmt_elf.o, and
> they are just two symbols that happen to share the same name, as is common
> in the kernel.
> 
> at the same time, addr2line reports symbols to be generated from the same
> file.
> 
> ```
> ~ $ llvm-addr2line-14 -fe vmlinux ffff8000082f1d2c ffff8000082f4454
> set_brk
> /home/alessandro/src/lka64/linux-6.4/fs/binfmt_elf.c:114
> set_brk
> /home/alessandro/src/lka64/linux-6.4/fs/binfmt_elf.c:114
> ~ $
> ~ $ addr2line -fe vmlinux ffff8000082f1d2c ffff8000082f4454
> set_brk
> /home/alessandro/src/lka64/linux-6.4/fs/binfmt_elf.c:114
> set_brk
> /home/alessandro/src/lka64/linux-6.4/fs/binfmt_elf.c:114
> ```
> looking at the source code:
> https://elixir.bootlin.com/linux/v6.4/source/fs/compat_binfmt_elf.c#L144
> the cause of this behavior, which is unexpected but correct.

Thank you for the investigation, this an interesting reading!

> The rationale is that using source file + line number iproduces better
> kallsyms table, but it is still do not produce unique names.

When I first read your cover letter, I also thought it would be cool to have 
the filename rather than a serial ID.
So, your latest modification is really good as it goes into this direction.
Regarding having non unique names, I am not sure if this is a problem.
In my case, which is using kprobe to attach BPF code to trace some events, 
your contribution would make me on the way to know which address I should use 
because I could choose regarding the filename.

I am curious about a potential v3 of this patch!

> BR
> Alessandro
> 
> > As a final note, please understand that my patch was not intended to
> > undermine anyone's work. I simply encountered a problem that I wanted to
> > help solve. Attached to this message is my code, in case anyone wants to
> > replicate it. I would appreciate being kept in the loop, as I genuinely
> > want to assist in fixing this problem.
> > 
> > BR
> > Alessandro
> > ---
> > 
> >  init/Kconfig                        |  36 ++++
> >  scripts/Makefile                    |   4 +
> >  scripts/kas_alias/Makefile          |   4 +
> >  scripts/kas_alias/a2l.c             | 268 ++++++++++++++++++++++++++++
> >  scripts/kas_alias/a2l.h             |  32 ++++
> >  scripts/kas_alias/duplicates_list.c |  70 ++++++++
> >  scripts/kas_alias/duplicates_list.h |  15 ++
> >  scripts/kas_alias/item_list.c       | 230 ++++++++++++++++++++++++
> >  scripts/kas_alias/item_list.h       |  26 +++
> >  scripts/kas_alias/kas_alias.c       | 217 ++++++++++++++++++++++
> >  scripts/link-vmlinux.sh             |  11 +-
> >  11 files changed, 910 insertions(+), 3 deletions(-)
> >  create mode 100644 scripts/kas_alias/Makefile
> >  create mode 100644 scripts/kas_alias/a2l.c
> >  create mode 100644 scripts/kas_alias/a2l.h
> >  create mode 100644 scripts/kas_alias/duplicates_list.c
> >  create mode 100644 scripts/kas_alias/duplicates_list.h
> >  create mode 100644 scripts/kas_alias/item_list.c
> >  create mode 100644 scripts/kas_alias/item_list.h
> >  create mode 100644 scripts/kas_alias/kas_alias.c
> > 
> > diff --git a/init/Kconfig b/init/Kconfig
> > index f7f65af4ee12..bc69fcd9cbc8 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -1737,6 +1737,42 @@ config KALLSYMS_BASE_RELATIVE
> > 
> >           time constants, and no relocation pass is required at runtime to
> >           fix
> >           up the entries based on the runtime load address of the kernel.
> > 
> > +config KALLSYMS_ALIAS
> > +       bool "Produces alias for duplicated symbols" if EXPERT
> > +       depends on KALLSYMS && (DEBUG_INFO_DWARF4 || DEBUG_INFO_DWARF5)
> > +       help
> > +         It is not uncommon for drivers or modules related to similar
> > +         peripherals to have symbols with the exact same name.
> > +         While this is not a problem for the kernel's binary itself, it
> > +         becomes an issue when attempting to trace or probe specific
> > +         functions using infrastructure like ftrace or kprobe.
> > +
> > +         This option addresses this challenge by extending the symbol
> > names +         with unique suffixes during the kernel build process.
> > +         The newly created aliases for these duplicated symbols are
> > unique
> > +         names that can be fed to the ftrace sysfs interface. By doing
> > so, it +         enables previously unreachable symbols to be probed.
> > +
> > +config CONFIG_KALLSYMS_ALIAS_DATA
> > +       bool "Produces alias also for data"
> > +       depends on KALLSYMS_ALIAS
> > +       help
> > +         Sometimes it can be useful to refer to data. In live patch
> > scenarios, +         you may find yourself needing to use symbols that
> > are shared with +         other functions. Since symbols face the same
> > issue as functions, this +         option allows you to create aliases
> > for data as well.
> > +
> > +config CONFIG_KALLSYMS_ALIAS_DATA_ALL
> > +       bool "Removes all filter when producing data alias"
> > +       depends on CONFIG_KALLSYMS_ALIAS_DATA
> > +       help
> > +         When selecting data aliases, not all symbols are included in the
> > set +         This is because many symbols are unlikely to be used. If
> > you choose +         to have an alias for all data symbols, be aware that
> > it will +         significantly increase the size.
> > +
> > +         If unsure, say N.
> > +
> > 
> >  # end of the "standard kernel features (expert users)" menu
> >  
> >  # syscall, maps, verifier
> > 
> > diff --git a/scripts/Makefile b/scripts/Makefile
> > index 32b6ba722728..65fafe17cfe5 100644
> > --- a/scripts/Makefile
> > +++ b/scripts/Makefile
> > @@ -49,3 +49,7 @@ subdir-$(CONFIG_SECURITY_SELINUX) += selinux
> > 
> >  # Let clean descend into subdirs
> >  subdir-        += basic dtc gdb kconfig mod
> > 
> > +
> > +# KALLSyms alias
> > +subdir-$(CONFIG_KALLSYMS_ALIAS) += kas_alias
> > +
> > diff --git a/scripts/kas_alias/Makefile b/scripts/kas_alias/Makefile
> > new file mode 100644
> > index 000000000000..e1fde69232b4
> > --- /dev/null
> > +++ b/scripts/kas_alias/Makefile
> > @@ -0,0 +1,4 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +hostprogs-always-$(CONFIG_KALLSYMS_ALIAS)    += kas_alias
> > +
> > +kas_alias-objs        := duplicates_list.o item_list.o kas_alias.o a2l.o
> > diff --git a/scripts/kas_alias/a2l.c b/scripts/kas_alias/a2l.c
> > new file mode 100644
> > index 000000000000..a9692ac30180
> > --- /dev/null
> > +++ b/scripts/kas_alias/a2l.c
> > @@ -0,0 +1,268 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <unistd.h>
> > +#include <sys/types.h>
> > +#include <sys/wait.h>
> > +#include <string.h>
> > +#include <stdint.h>
> > +#include <stdbool.h>
> > +
> > +#include "a2l.h"
> > +
> > +int addr2line_pid = -1;
> > +int a2l_in[2];
> > +int a2l_out[2];
> > +char line[MAX_BUF];
> > +char vmlinux_path[MAX_BUF];
> > +char addr2line_cmd[MAX_CMD_LEN];
> > +FILE *a2l_stdin, *a2l_stdout;
> > +
> > +static char *normalize_path(const char *input_path, char *output_path)
> > +{
> > +       char *prev_token = NULL;
> > +       char *delimiter = "/";
> > +       char inbuf[MAX_BUF];
> > +       char *token;
> > +       char *pos;
> > +
> > +       memset(inbuf, 0, MAX_BUF);
> > +       *output_path = '\0';
> > +       strncpy(inbuf, input_path, MAX_BUF);
> > +       if (!input_path || !output_path || strlen(input_path) == 0)
> > +               return NULL;
> > +
> > +       token = strtok(inbuf, delimiter);
> > +       while (token) {
> > +               if (strcmp(token, "..") == 0 && prev_token) {
> > +                       pos = strrchr(output_path, '/');
> > +                       if (pos)
> > +                               *pos = '\0';
> > +
> > +               } else if (strcmp(token, ".") != 0) {
> > +                       strcat(output_path, "/");
> > +                       strcat(output_path, token);
> > +               }
> > +
> > +               prev_token = token;
> > +               token = strtok(NULL, delimiter);
> > +       }
> > +
> > +       return output_path;
> > +}
> > +
> > +static void path_of(const char *full_path, char *path)
> > +{
> > +       const char *last_slash = strrchr(full_path, '/');
> > +       size_t path_length;
> > +       char cwd[MAX_BUF];
> > +
> > +       if (!last_slash) {
> > +               if (getcwd(cwd, sizeof(cwd)))
> > +                       strcpy(path, cwd);
> > +               else
> > +                       strcpy(path, ".");
> > +       } else {
> > +               path_length = last_slash - full_path;
> > +               strncpy(path, full_path, path_length);
> > +               path[path_length] = '\0';
> > +       }
> > +}
> > +
> > +static bool file_exists(const char *file_path)
> > +{
> > +       FILE *file;
> > +
> > +       file = fopen(file_path, "r");
> > +       if (file) {
> > +               fclose(file);
> > +               return true;
> > +       }
> > +       return false;
> > +}
> > +
> > +int addr2line_init(const char *cmd, const char *vmlinux)
> > +{
> > +       if ((!file_exists(cmd)) || (!file_exists(vmlinux))) {
> > +               printf("file not found\n");
> > +               return 0;
> > +               }
> > +
> > +       path_of(vmlinux, vmlinux_path);
> > +       if (pipe(a2l_in) == -1) {
> > +               printf("Failed to create pipe\n");
> > +               return 0;
> > +       }
> > +
> > +       if (pipe(a2l_out) == -1) {
> > +               printf("Failed to create pipe\n");
> > +               return 0;
> > +       }
> > +
> > +       addr2line_pid = fork();
> > +       if (addr2line_pid == -1) {
> > +               printf("Failed to fork process\n");
> > +               close(a2l_in[P_READ]);
> > +               close(a2l_in[P_WRITE]);
> > +               close(a2l_out[P_READ]);
> > +               close(a2l_out[P_WRITE]);
> > +               return 0;
> > +       }
> > +
> > +       if (addr2line_pid == 0) {
> > +               dup2(a2l_in[P_READ], 0);
> > +               dup2(a2l_out[P_WRITE], 1);
> > +               close(a2l_in[P_WRITE]);
> > +               close(a2l_out[P_READ]);
> > +
> > +               execlp(cmd, cmd, ADDR2LINE_ARGS, vmlinux, NULL);
> > +
> > +               printf("Failed to execute addr2line command\n");
> > +               exit(1);
> > +       } else {
> > +               close(a2l_in[P_READ]);
> > +               close(a2l_out[P_WRITE]);
> > +       }
> > +
> > +       a2l_stdin = fdopen(a2l_in[P_WRITE], "w");
> > +       if (!a2l_stdin) {
> > +               printf("Failed to open pipe a2l_in\n");
> > +               return 0;
> > +       }
> > +
> > +       a2l_stdout = fdopen(a2l_out[P_READ], "r");
> > +       if (!a2l_stdout) {
> > +               printf("Failed to open pipe a2l_out\n");
> > +               fclose(a2l_stdin);
> > +               return 0;
> > +       }
> > +
> > +       return 1;
> > +}
> > +
> > +const char *remove_subdir(const char *home, const char *f_path)
> > +{
> > +       int i = 0;
> > +
> > +       while (*(home + i) == *(f_path + i))
> > +               i++;
> > +
> > +       return (strlen(home) != i) ? NULL : f_path + i;
> > +}
> > +
> > +char *addr2line_get_lines(uint64_t address)
> > +{
> > +       char buf[MAX_BUF];
> > +
> > +       fprintf(a2l_stdin, "%08lx\n", address);
> > +       fflush(a2l_stdin);
> > +
> > +       if (!fgets(line, sizeof(line), a2l_stdout)) {
> > +               printf("Failed to read lines from addr2line\n");
> > +               return NULL;
> > +       }
> > +
> > +       if (!fgets(line, sizeof(line), a2l_stdout)) {
> > +               printf("Failed to read lines from addr2line\n");
> > +               return NULL;
> > +       }
> > +
> > +       line[strcspn(line, "\n")] = '\0';
> > +       strncpy(buf, line, MAX_BUF);
> > +       return normalize_path(buf, line);
> > +}
> > +
> > +int addr2line_cleanup(void)
> > +{
> > +       int status;
> > +
> > +       if (addr2line_pid != -1) {
> > +               kill(addr2line_pid, SIGKILL);
> > +               waitpid(addr2line_pid, &status, 0);
> > +               fclose(a2l_stdin);
> > +               fclose(a2l_stdout);
> > +               addr2line_pid = -1;
> > +       }
> > +
> > +       return 1;
> > +}
> > +
> > +static char *find_executable(const char *command)
> > +{
> > +       char *path_env = getenv("PATH");
> > +       char *executable_path;
> > +       char *path_copy;
> > +       char *path;
> > +       int n;
> > +
> > +       if (!path_env)
> > +               return NULL;
> > +
> > +       path_copy = strdup(path_env);
> > +       if (!path_copy)
> > +               return NULL;
> > +
> > +       path = strtok(path_copy, ":");
> > +       while (path) {
> > +               n = snprintf(0, 0, "%s/%s", path, command);
> > +               executable_path = (char *)malloc(n + 1);
> > +               snprintf(executable_path, n + 1, "%s/%s", path, command);
> > +               if (access(executable_path, X_OK) == 0) {
> > +                       free(path_copy);
> > +                       return executable_path;
> > +               }
> > +
> > +       path = strtok(NULL, ":");
> > +       free(executable_path);
> > +       executable_path = NULL;
> > +       }
> > +
> > +       free(path_copy);
> > +       if (executable_path)
> > +               free(executable_path);
> > +       return NULL;
> > +}
> > +
> > +const char *get_addr2line(int mode)
> > +{
> > +       char *buf = "";
> > +
> > +       switch (mode) {
> > +       case A2L_CROSS:
> > +               buf = getenv("CROSS_COMPILE");
> > +               memcpy(addr2line_cmd, buf, strlen(buf));
> > +       case A2L_DEFAULT:
> > +               memcpy(addr2line_cmd + strlen(buf), ADDR2LINE,
> > strlen(ADDR2LINE)); +               buf = find_executable(addr2line_cmd);
> > +               if (buf) {
> > +                       memcpy(addr2line_cmd, buf, strlen(buf));
> > +                       free(buf);
> > +               }
> > +               return addr2line_cmd;
> > +       case A2L_LLVM:
> > +       default:
> > +               return NULL;
> > +       }
> > +}
> > +
> > +char *get_vmlinux(char *input)
> > +{
> > +       const char *match_string1 = ".syms";
> > +       const char *match_string2 = ".tmp_vmlinux.kallsyms";
> > +       char *result = NULL;
> > +       char *match_pos;
> > +
> > +       match_pos = strstr(input, match_string1);
> > +       if (!match_pos)
> > +               return NULL;
> > +
> > +       match_pos = strstr(input, match_string2);
> > +       if (!match_pos)
> > +               return NULL;
> > +
> > +       result = strdup(input);
> > +       match_pos = strstr(result, match_string1);
> > +       *match_pos = '\0';
> > +       return result;
> > +}
> > diff --git a/scripts/kas_alias/a2l.h b/scripts/kas_alias/a2l.h
> > new file mode 100644
> > index 000000000000..ca6419229dde
> > --- /dev/null
> > +++ b/scripts/kas_alias/a2l.h
> > @@ -0,0 +1,32 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +#ifndef A2L_H
> > +#define A2L_H
> > +#include <stdint.h>
> > +
> > +#define ADDR2LINE "addr2line"
> > +#define ADDR2LINE_ARGS "-fe"
> > +//#define VMLINUX "vmlinux"
> > +#define MAX_BUF 4096
> > +#define MAX_CMD_LEN 256
> > +#define P_READ 0
> > +#define P_WRITE 1
> > +#define A2L_DEFAULT 1
> > +#define A2L_CROSS 2
> > +#define A2L_LLVM 3
> > +#define A2L_MAKE_VALUE 2
> > +
> > +extern int addr2line_pid;
> > +extern int a2l_in[2];
> > +extern int a2l_out[2];
> > +extern char line[MAX_BUF];
> > +extern char vmlinux_path[MAX_BUF];
> > +extern char addr2line_cmd[MAX_CMD_LEN];
> > +
> > +int addr2line_init(const char *cmd, const char *vmlinux);
> > +char *addr2line_get_lines(uint64_t address);
> > +int addr2line_cleanup(void);
> > +const char *remove_subdir(const char *home, const char *f_path);
> > +const char *get_addr2line(int mode);
> > +char *get_vmlinux(char *input);
> > +
> > +#endif
> > diff --git a/scripts/kas_alias/duplicates_list.c
> > b/scripts/kas_alias/duplicates_list.c new file mode 100644
> > index 000000000000..e7a3d2917937
> > --- /dev/null
> > +++ b/scripts/kas_alias/duplicates_list.c
> > @@ -0,0 +1,70 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +#include <stdint.h>
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include <stdlib.h>
> > +#include <stdbool.h>
> > +
> > +#include "item_list.h"
> > +#include "duplicates_list.h"
> > +
> > +struct duplicate_item *find_duplicates(struct item *list)
> > +{
> > +       struct duplicate_item *current_duplicate = NULL;
> > +       struct duplicate_item *duplicates = NULL;
> > +       struct duplicate_item *new_duplicate;
> > +       struct item *current_item = list;
> > +       bool prev_was_duplicate = false;
> > +       struct item *prev_item = NULL;
> > +
> > +       while (current_item) {
> > +               if ((prev_item && (strcmp(current_item->symb_name,
> > prev_item->symb_name) == 0)) || +                   prev_was_duplicate) {
> > +                       if (!duplicates) {
> > +                               duplicates = malloc(sizeof(struct
> > duplicate_item)); +                               if (!duplicates)
> > +                                       return NULL;
> > +
> > +                               duplicates->original_item = prev_item;
> > +                               duplicates->next = NULL;
> > +                               current_duplicate = duplicates;
> > +                       } else {
> > +                               new_duplicate = malloc(sizeof(struct
> > duplicate_item)); +                               if (!new_duplicate) {
> > +                                       free_duplicates(&duplicates);
> > +                                       return NULL;
> > +                               }
> > +
> > +                               new_duplicate->original_item = prev_item;
> > +                               new_duplicate->next = NULL;
> > +                               current_duplicate->next = new_duplicate;
> > +                               current_duplicate = new_duplicate;
> > +
> > +                               if ((strcmp(current_item->symb_name,
> > prev_item->symb_name) != 0) && +                                  
> > (prev_was_duplicate))
> > +                                       prev_was_duplicate = false;
> > +                               else
> > +                                       prev_was_duplicate = true;
> > +                       }
> > +               }
> > +
> > +               prev_item = current_item;
> > +               current_item = current_item->next;
> > +       }
> > +
> > +       return duplicates;
> > +}
> > +
> > +void free_duplicates(struct duplicate_item **duplicates)
> > +{
> > +       struct duplicate_item *duplicates_iterator = *duplicates;
> > +       struct duplicate_item *app;
> > +
> > +       while (duplicates_iterator) {
> > +               app = duplicates_iterator;
> > +               duplicates_iterator = duplicates_iterator->next;
> > +               free(app);
> > +       }
> > +
> > +       *duplicates = NULL;
> > +}
> > diff --git a/scripts/kas_alias/duplicates_list.h
> > b/scripts/kas_alias/duplicates_list.h new file mode 100644
> > index 000000000000..76aa73e584bc
> > --- /dev/null
> > +++ b/scripts/kas_alias/duplicates_list.h
> > @@ -0,0 +1,15 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +#ifndef DUPLICATES_LIST_H
> > +#define DUPLICATES_LIST_H
> > +
> > +#include "item_list.h"
> > +
> > +struct duplicate_item {
> > +       struct item *original_item;
> > +       struct duplicate_item *next;
> > +};
> > +
> > +struct duplicate_item *find_duplicates(struct item *list);
> > +void free_duplicates(struct duplicate_item **duplicates);
> > +
> > +#endif
> > diff --git a/scripts/kas_alias/item_list.c b/scripts/kas_alias/item_list.c
> > new file mode 100644
> > index 000000000000..48f2e525592a
> > --- /dev/null
> > +++ b/scripts/kas_alias/item_list.c
> > @@ -0,0 +1,230 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <stdint.h>
> > +#include <string.h>
> > +#include <stdbool.h>
> > +#include <assert.h>
> > +#include "item_list.h"
> > +
> > +#define CHECK_ORDER_BY_ADDRESS(sort_by, current, temp, op) \
> > +       ((sort_by) == BY_ADDRESS && (current)->addr op (temp)->addr)
> > +#define CHECK_ORDER_BY_NAME(sort_by, current, temp, op) \
> > +       ((sort_by) == BY_NAME && strcmp((current)->symb_name,
> > (temp)->symb_name) op 0) +
> > +struct item *list_index[96] = {0};
> > +
> > +void build_index(struct item *list)
> > +{
> > +       char current_first_letter = ' ';
> > +       struct item *current = list;
> > +
> > +       while (current) {
> > +               if (current->symb_name[0] != current_first_letter) {
> > +                       current_first_letter = current->symb_name[0];
> > +                       list_index[current_first_letter - 32] = current;
> > +               }
> > +               current = current->next;
> > +       }
> > +}
> > +
> > +struct item *add_item(struct item **list, const char *name, char stype,
> > uint64_t addr) +{
> > +       struct item *new_item;
> > +       struct item *current;
> > +
> > +       new_item = malloc(sizeof(struct item));
> > +       if (!new_item)
> > +               return NULL;
> > +
> > +       strncpy(new_item->symb_name, name, MAX_NAME_SIZE);
> > +       new_item->symb_name[MAX_NAME_SIZE - 1] = '\0';
> > +       new_item->addr = addr;
> > +       new_item->stype = stype;
> > +       new_item->next = NULL;
> > +
> > +       if (!(*list)) {
> > +               *list = new_item;
> > +       } else {
> > +               current = *list;
> > +               while (current->next)
> > +                       current = current->next;
> > +
> > +               current->next = new_item;
> > +       }
> > +       return new_item;
> > +}
> > +
> > +void sort_list(struct item **list, int sort_by)
> > +{
> > +       struct item *current = *list;
> > +       struct item *sorted = NULL;
> > +       struct item *next_item;
> > +       struct item *temp;
> > +
> > +       if (!(*list) || !((*list)->next))
> > +               return;
> > +
> > +       while (current) {
> > +               next_item = current->next;
> > +               if (!sorted ||
> > +                   (CHECK_ORDER_BY_ADDRESS(sort_by, current, sorted, <)
> > ||
> > +                   CHECK_ORDER_BY_NAME(sort_by, current, sorted, >=))) {
> > +                       current->next = sorted;
> > +                       sorted = current;
> > +               } else {
> > +                       temp = sorted;
> > +                       while (temp->next &&
> > +                              (CHECK_ORDER_BY_ADDRESS(sort_by, current,
> > temp->next, >=) || +                             
> > CHECK_ORDER_BY_NAME(sort_by, current, temp->next, >=))) +                
> >               temp = temp->next;
> > +
> > +                       current->next = temp->next;
> > +                       temp->next = current;
> > +               }
> > +               current = next_item;
> > +       }
> > +
> > +       *list = sorted;
> > +}
> > +
> > +struct item *merge(struct item *left, struct item *right, int sort_by)
> > +{
> > +       struct item *current = NULL;
> > +       struct item *result = NULL;
> > +
> > +       if (!left)
> > +               return right;
> > +       if (!right)
> > +               return left;
> > +
> > +       if (sort_by == BY_NAME) {
> > +               if (strcmp(left->symb_name, right->symb_name) <= 0) {
> > +                       result = left;
> > +                       left = left->next;
> > +               } else {
> > +                       result = right;
> > +                       right = right->next;
> > +               }
> > +       } else {
> > +               if (sort_by == BY_ADDRESS) {
> > +                       if (left->addr <= right->addr) {
> > +                               result = left;
> > +                               left = left->next;
> > +                       } else {
> > +                               result = right;
> > +                               right = right->next;
> > +                       }
> > +               }
> > +       }
> > +
> > +       current = result;
> > +
> > +       while (left && right) {
> > +               if (sort_by == BY_NAME) {
> > +                       if (strcmp(left->symb_name, right->symb_name) <=
> > 0) { +                               current->next = left;
> > +                               left = left->next;
> > +                       } else {
> > +                               current->next = right;
> > +                               right = right->next;
> > +                       }
> > +               } else {
> > +                       if (sort_by == BY_ADDRESS) {
> > +                               if (left->addr <= right->addr) {
> > +                                       current->next = left;
> > +                                       left = left->next;
> > +                               } else {
> > +                                       current->next = right;
> > +                                       right = right->next;
> > +                               }
> > +                       }
> > +               }
> > +
> > +               current = current->next;
> > +       }
> > +
> > +       if (left) {
> > +               current->next = left;
> > +       } else {
> > +               if (right)
> > +                       current->next = right;
> > +       }
> > +
> > +       return result;
> > +}
> > +
> > +struct item *merge_sort(struct item *head, int sort_by)
> > +{
> > +       struct item *right;
> > +       struct item *slow;
> > +       struct item *fast;
> > +       struct item *left;
> > +
> > +       if (!head || !head->next)
> > +               return head;
> > +
> > +       slow = head;
> > +       fast = head->next;
> > +
> > +       while (fast && fast->next) {
> > +               slow = slow->next;
> > +               fast = fast->next->next;
> > +       }
> > +
> > +       left = head;
> > +       right = slow->next;
> > +       slow->next = NULL;
> > +
> > +       left = merge_sort(left, sort_by);
> > +       right = merge_sort(right, sort_by);
> > +
> > +       return merge(left, right, sort_by);
> > +}
> > +
> > +void sort_list_m(struct item **head, int sort_by)
> > +{
> > +       if (!(*head) || !((*head)->next))
> > +               return;
> > +
> > +       *head = merge_sort(*head, sort_by);
> > +}
> > +
> > +int insert_after(struct item *list, const uint64_t search_addr,
> > +                const char *name, uint64_t addr, char stype)
> > +{
> > +       struct item *new_item;
> > +       struct item *current;
> > +       int ret = 0;
> > +
> > +       current = (list_index[name[0] - 32]) ? list_index[name[0] - 32] :
> > list; +       while (current) {
> > +               if (current->addr == search_addr) {
> > +                       new_item = malloc(sizeof(struct item));
> > +                       if (!new_item)
> > +                               return ret;
> > +                       strncpy(new_item->symb_name, name, MAX_NAME_SIZE);
> > +                       new_item->symb_name[MAX_NAME_SIZE - 1] = '\0';
> > +                       new_item->addr = addr;
> > +                       new_item->stype = stype;
> > +                       new_item->next = current->next;
> > +                       current->next = new_item;
> > +                       ret = 1;
> > +                       break;
> > +               }
> > +               current = current->next;
> > +       }
> > +       return ret;
> > +}
> > +
> > +void free_items(struct item **head)
> > +{
> > +       struct item *app, *item_iterator = *head;
> > +
> > +       while (item_iterator) {
> > +               app = item_iterator;
> > +               item_iterator = item_iterator->next;
> > +               free(app);
> > +       }
> > +       *head = NULL;
> > +}
> > diff --git a/scripts/kas_alias/item_list.h b/scripts/kas_alias/item_list.h
> > new file mode 100644
> > index 000000000000..b4891cb088ee
> > --- /dev/null
> > +++ b/scripts/kas_alias/item_list.h
> > @@ -0,0 +1,26 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +#ifndef ITEM_LIST_H
> > +#define ITEM_LIST_H
> > +#include <stdint.h>
> > +
> > +#define MAX_NAME_SIZE 256
> > +#define BY_ADDRESS 1
> > +#define BY_NAME 2
> > +
> > +struct item {
> > +       char            symb_name[MAX_NAME_SIZE];
> > +       uint64_t        addr;
> > +       char            stype;
> > +       struct item     *next;
> > +};
> > +
> > +void build_index(struct item *list);
> > +struct item *add_item(struct item **list, const char *name, char stype,
> > uint64_t addr); +void sort_list(struct item **list, int sort_by);
> > +struct item *merge(struct item *left, struct item *right, int sort_by);
> > +struct item *merge_sort(struct item *head, int sort_by);
> > +void sort_list_m(struct item **head, int sort_by);
> > +int insert_after(struct item *list, const uint64_t search_addr,
> > +                const char *name, uint64_t addr, char stype);
> > +void free_items(struct item **head);
> > +#endif
> > diff --git a/scripts/kas_alias/kas_alias.c b/scripts/kas_alias/kas_alias.c
> > new file mode 100644
> > index 000000000000..532aeb39f851
> > --- /dev/null
> > +++ b/scripts/kas_alias/kas_alias.c
> > @@ -0,0 +1,217 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <stdint.h>
> > +#include <unistd.h>
> > +#include <string.h>
> > +#include <stdbool.h>
> > +#include <stdarg.h>
> > +#include <regex.h>
> > +
> > +#include "item_list.h"
> > +#include "duplicates_list.h"
> > +#include "a2l.h"
> > +
> > +#define SYMB_IS_TEXT(s) ((((s)->stype) == 't') ||  (((s)->stype) == 'T'))
> > +#define SYMB_IS_DATA(s) ((((s)->stype) == 'b') ||  (((s)->stype) == 'B')
> > || \ +                        (((s)->stype) == 'd') ||  (((s)->stype) ==
> > 'D') || \ +                        (((s)->stype) == 'r') || 
> > (((s)->stype) == 'R')) +#ifdef CONFIG_KALLSYMS_ALIAS_DATA
> > +#define SYMB_NEEDS_ALIAS(s) (SYMB_IS_TEXT(s) || SYMB_IS_DATA(s))
> > +#else
> > +#define SYMB_NEEDS_ALIAS(s) SYMB_IS_TEXT(s)
> > +#endif
> > +#define FNOMATCH 0
> > +#define FMATCH 1
> > +#define EREGEX 2
> > +
> > +const char *ignore_regex[] = {
> > +       "^__cfi_.*$",                           // __cfi_ preamble
> > +#ifndef CONFIG_KALLSYMS_ALIAS_DATA_ALL
> > +       "^_*TRACE_SYSTEM.*$",
> > +       "^__already_done\\.[0-9]+$",            // Call a function once
> > data +       "^___tp_str\\.[0-9]+$",
> > +       "^___done\\.[0-9]+$",
> > +       "^__print_once\\.[0-9]+$",
> > +       "^_rs\\.[0-9]+$",
> > +       "^__compound_literal\\.[0-9]+$",
> > +       "^___once_key\\.[0-9]+$",
> > +       "^__func__\\.[0-9]+$",
> > +       "^__msg\\.[0-9]+$",
> > +       "^CSWTCH\\.[0-9]+$",
> > +       "^__flags\\.[0-9]+$",
> > +       "^__wkey.*$",
> > +       "^__mkey.*$",
> > +       "^__key.*$",
> > +#endif
> > +       "^__pfx_.*$"                            // NOP-padding
> > +};
> > +
> > +int suffix_serial;
> > +
> > +static inline void verbose_msg(bool verbose, const char *fmt, ...)
> > +{
> > +       va_list args;
> > +
> > +       va_start(args, fmt);
> > +       if (verbose)
> > +               printf(fmt, args);
> > +
> > +       va_end(args);
> > +}
> > +
> > +static void create_suffix(const char *name, char *output_suffix)
> > +{
> > +       sprintf(output_suffix, "%s__alias__%d", name, suffix_serial++);
> > +}
> > +
> > +static void create_file_suffix(const char *name, uint64_t address, char
> > *output_suffix, char *cwd) +{
> > +       const char *f_path;
> > +       char *buf;
> > +       int i = 0;
> > +
> > +       buf = addr2line_get_lines(address);
> > +       f_path = remove_subdir(cwd, buf);
> > +       if (f_path) {
> > +               sprintf(output_suffix, "%s@%s", name, f_path);
> > +               while (*(output_suffix + i) != '\0') {
> > +                       switch (*(output_suffix + i)) {
> > +                       case '/':
> > +                       case ':':
> > +                       case '.':
> > +                               *(output_suffix + i) = '_';
> > +                               break;
> > +                       default:
> > +                       }
> > +               i++;
> > +               }
> > +       } else {
> > +               create_suffix(name, output_suffix);
> > +       }
> > +}
> > +
> > +static int filter_symbols(char *symbol, const char **ignore_list, int
> > regex_no) +{
> > +       regex_t regex;
> > +       int res, i;
> > +
> > +       for (i = 0; i < regex_no; i++) {
> > +               res = regcomp(&regex, ignore_list[i], REG_EXTENDED);
> > +               if (res)
> > +                       return -EREGEX;
> > +
> > +               res = regexec(&regex, symbol, 0, NULL, 0);
> > +               regfree(&regex);
> > +               switch (res) {
> > +               case 0:
> > +                       return FMATCH;
> > +               case REG_NOMATCH:
> > +                       break;
> > +               default:
> > +                       return -EREGEX;
> > +               }
> > +       }
> > +
> > +       return FNOMATCH;
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +       char t, sym_name[MAX_NAME_SIZE], new_name[MAX_NAME_SIZE + 15];
> > +       struct duplicate_item  *duplicate_iterator;
> > +       struct duplicate_item *duplicate;
> > +       struct item *head = {NULL};
> > +       bool need_2_process = true;
> > +       struct item *last = {NULL};
> > +       struct item  *current;
> > +       int verbose_mode = 0;
> > +       uint64_t address;
> > +       FILE *fp;
> > +       int res;
> > +
> > +       if (argc < 2 || argc > 3) {
> > +               printf("Usage: %s <nmfile> [-verbose]\n", argv[0]);
> > +               return 1;
> > +       }
> > +
> > +       if (argc == 3 && strcmp(argv[2], "-verbose") == 0)
> > +               verbose_mode = 1;
> > +
> > +       verbose_msg(verbose_mode, "Scanning nm data(%s)\n", argv[1]);
> > +
> > +       fp = fopen(argv[1], "r");
> > +       if (!fp) {
> > +               printf("Can't open input file.\n");
> > +               return 1;
> > +       }
> > +
> > +       if (!addr2line_init(get_addr2line(A2L_DEFAULT),
> > get_vmlinux(argv[1]))) +               return 1;
> > +
> > +       while (fscanf(fp, "%lx %c %99s\n", &address, &t, sym_name) == 3) {
> > +               if (strstr(sym_name, "@_")) {
> > +                       if (verbose_mode && need_2_process)
> > +                               printf("Already processed\n");
> > +                       need_2_process = false;
> > +                       }
> > +               last = add_item(&last, sym_name, t, address);
> > +               if (!last) {
> > +                       printf("Error in allocate memory\n");
> > +                       free_items(&head);
> > +                       return 1;
> > +               }
> > +
> > +               if (!head)
> > +                       head = last;
> > +       }
> > +
> > +       fclose(fp);
> > +
> > +       if (need_2_process) {
> > +               verbose_msg(verbose_mode, "Sorting nm data\n");
> > +               sort_list_m(&head, BY_NAME);
> > +               verbose_msg(verbose_mode, "Scanning nm data for
> > duplicates\n"); +               duplicate = find_duplicates(head);
> > +               if (!duplicate) {
> > +                       printf("Error in duplicates list\n");
> > +                       return 1;
> > +               }
> > +
> > +               verbose_msg(verbose_mode, "Applying suffixes\n");
> > +               build_index(head);
> > +               duplicate_iterator = duplicate;
> > +               while (duplicate_iterator) {
> > +                       res =
> > filter_symbols(duplicate_iterator->original_item->symb_name, +           
> >                                 ignore_regex, sizeof(ignore_regex) / +   
> >                                         sizeof(ignore_regex[0])); +      
> >                 if (res != FMATCH &&
> > +                          
> > SYMB_NEEDS_ALIAS(duplicate_iterator->original_item)) { +                 
> >              if (res < 0)
> > +                                       return 1;
> > +
> > +                              
> > create_file_suffix(duplicate_iterator->original_item->symb_name, +       
> >                                          
> > duplicate_iterator->original_item->addr, +                               
> >                   new_name, vmlinux_path); +                             
> >  if (!insert_after(head, duplicate_iterator->original_item->addr, +      
> >                                           new_name,
> > duplicate_iterator->original_item->addr, +                               
> >                  duplicate_iterator->original_item->stype)) +            
> >                           return 1;
> > +                       }
> > +
> > +                       duplicate_iterator = duplicate_iterator->next;
> > +               }
> > +
> > +               sort_list_m(&head, BY_ADDRESS);
> > +       }
> > +       current = head;
> > +       while (current) {
> > +               printf("%08lx %c %s\n", current->addr, current->stype,
> > current->symb_name); +               current = current->next;
> > +       }
> > +
> > +       free_items(&head);
> > +       free_duplicates(&duplicate);
> > +       addr2line_cleanup();
> > +       return 0;
> > +}
> > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > index a432b171be82..cacf60b597ce 100755
> > --- a/scripts/link-vmlinux.sh
> > +++ b/scripts/link-vmlinux.sh
> > @@ -89,8 +89,9 @@ vmlinux_link()
> > 
> >         ldflags="${ldflags} ${wl}--script=${objtree}/${KBUILD_LDS}"
> > 
> > -       # The kallsyms linking does not need debug symbols included.
> > -       if [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
> > +       # The kallsyms linking does not need debug symbols included,
> > unless the KALLSYMS_ALIAS. +       if [ ! is_enabled
> > CONFIG_KALLSYMS_ALIAS ] && \
> > +           [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
> > 
> >                 ldflags="${ldflags} ${wl}--strip-debug"
> >         
> >         fi
> > 
> > @@ -161,7 +162,11 @@ kallsyms()
> > 
> >         fi
> >         
> >         info KSYMS ${2}
> > 
> > -       scripts/kallsyms ${kallsymopt} ${1} > ${2}
> > +       if is_enabled CONFIG_KALLSYMS_ALIAS; then
> > +               ALIAS=".alias"
> > +               scripts/kas_alias/kas_alias ${1} >${1}${ALIAS}
> > +               fi
> > +       scripts/kallsyms ${kallsymopt} ${1}${ALIAS} > ${2}
> > 
> >  }
> >  
> >  # Perform one step in kallsyms generation, including temporary linking of
> > 
> > --
> > 2.34.1

Best regards.



^ permalink raw reply

* Re: [PATCH RFC 00/37] Add support for arm64 MTE dynamic tag storage reuse
From: Catalin Marinas @ 2023-08-24 15:24 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Alexandru Elisei, will, oliver.upton, maz, james.morse,
	suzuki.poulose, yuzenghui, arnd, akpm, mingo, peterz, juri.lelli,
	vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman,
	bristot, vschneid, mhiramat, rppt, hughd, pcc, steven.price,
	anshuman.khandual, vincenzo.frascino, eugenis, kcc, hyesoo.yu,
	linux-arm-kernel, linux-kernel, kvmarm, linux-fsdevel, linux-arch,
	linux-mm, linux-trace-kernel
In-Reply-To: <0b9c122a-c05a-b3df-c69f-85f520294adc@redhat.com>

On Thu, Aug 24, 2023 at 01:25:41PM +0200, David Hildenbrand wrote:
> On 24.08.23 13:06, David Hildenbrand wrote:
> > On 24.08.23 12:44, Catalin Marinas wrote:
> > > The way MTE is implemented currently is to have a static carve-out of
> > > the DRAM to store the allocation tags (a.k.a. memory colour). This is
> > > what we call the tag storage. Each 16 bytes have 4 bits of tags, so this
> > > means 1/32 of the DRAM, roughly 3% used for the tag storage. This is
> > > done transparently by the hardware/interconnect (with firmware setup)
> > > and normally hidden from the OS. So a checked memory access to location
> > > X generates a tag fetch from location Y in the carve-out and this tag is
> > > compared with the bits 59:56 in the pointer. The correspondence from X
> > > to Y is linear (subject to a minimum block size to deal with some
> > > address interleaving). The software doesn't need to know about this
> > > correspondence as we have specific instructions like STG/LDG to location
> > > X that lead to a tag store/load to Y.
> > > 
> > > Now, not all memory used by applications is tagged (mmap(PROT_MTE)).
> > > For example, some large allocations may not use PROT_MTE at all or only
> > > for the first and last page since initialising the tags takes time. The
> > > side-effect is that of these 3% DRAM, only part, say 1% is effectively
> > > used. Some people want the unused tag storage to be released for normal
> > > data usage (i.e. give it to the kernel page allocator).
[...]
> > So it sounds like you might want to provide that tag memory using CMA.
> > 
> > That way, only movable allocations can end up on that CMA memory area,
> > and you can allocate selected tag pages on demand (similar to the
> > alloc_contig_range() use case).
> > 
> > That also solves the issue that such tag memory must not be longterm-pinned.
> > 
> > Regarding one complication: "The kernel needs to know where to allocate
> > a PROT_MTE page from or migrate a current page if it becomes PROT_MTE
> > (mprotect()) and the range it is in does not support tagging.",
> > simplified handling would be if it's in a MIGRATE_CMA pageblock, it
> > doesn't support tagging. You have to migrate to a !CMA page (for
> > example, not specifying GFP_MOVABLE as a quick way to achieve that).
> 
> Okay, I now realize that this patch set effectively duplicates some CMA
> behavior using a new migrate-type.

Yes, pretty much, with some additional hooks to trigger migration. The
CMA mechanism was a great source of inspiration.

In addition, there are some races that are addressed mostly around page
migration/copying: the source page is untagged, the destination
allocated as untagged but before the copy an mprotect() makes the source
tagged (PG_mte_tagged set) and the copy_highpage() mechanism not having
anywhere to store the tags.

> Yeah, that's probably not what we want just to identify if memory is
> taggable or not.
> 
> Maybe there is a way to just keep reusing most of CMA instead.

A potential issue is that devices (mobile phones) may need a different
CMA range as well for DMA (and not necessarily in ZONE_DMA). Can
free_area[MIGRATE_CMA] handle multiple disjoint ranges? I don't see why
not as it's just a list.

We (Google and Arm) went through a few rounds of discussions and
prototyping trying to find the best approach: (1) a separate free_area[]
array in each zone (early proof of concept from Peter C and Evgenii S,
https://github.com/google/sanitizers/tree/master/mte-dynamic-carveout),
(2) a new ZONE_METADATA, (3) a separate CPU-less NUMA node just for the
tag storage, (4) a new MIGRATE_METADATA type.

We settled on the latter as it closely resembles CMA without interfering
with it. I don't remember why we did not just go for MIGRATE_CMA, it may
have been the heterogeneous memory aspect and the fact that we don't
want PROT_MTE (VM_MTE) allocations from this range. If the hardware
allowed this, I think the patches would have been a bit simpler.

Alex can comment more next week on how we ended up with this choice but
if we find a way to avoid VM_MTE allocations from certain areas, I think
we can reuse the CMA infrastructure. A bigger hammer would be no VM_MTE
allocations from any CMA range but it seems too restrictive.

> Another simpler idea to get started would be to just intercept the first
> PROT_MTE, and allocate all CMA memory. In that case, systems that don't ever
> use PROT_MTE can have that additional 3% of memory.

We had this on the table as well but the most likely deployment, at
least initially, is only some secure services enabling MTE with various
apps gradually moving towards this in time. So that's why the main
pushback from vendors is having this 3% reserved permanently. Even if
all apps use MTE, only the anonymous mappings are PROT_MTE, so still not
fully using the tag storage.

-- 
Catalin

^ permalink raw reply

* Re: [RFC PATCH v2 1/1] tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols
From: Masami Hiramatsu @ 2023-08-24 14:47 UTC (permalink / raw)
  To: Francis Laniel; +Cc: linux-kernel, linux-trace-kernel, Steven Rostedt
In-Reply-To: <12271275.O9o76ZdvQC@pwmachine>

On Thu, 24 Aug 2023 16:31:13 +0200
Francis Laniel <flaniel@linux.microsoft.com> wrote:

> Hi.
> 
> Le jeudi 24 août 2023, 15:02:27 CEST Masami Hiramatsu a écrit :
> > On Thu, 24 Aug 2023 12:37:34 +0200
> > 
> > Francis Laniel <flaniel@linux.microsoft.com> wrote:
> > > 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
> > > call, because the effectively called one has no kprobes.
> > > 
> > > So, this commit returns EADDRNOTAVAIL when func matches several symbols.
> > > This way, user needs to use addr to remove the ambiguity.
> > 
> > Thanks for update the patch. I have some comments there.
> > 
> > > Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
> > > Link:
> > > https://lore.kernel.org/lkml/20230819101105.b0c104ae4494a7d1f2eea742@kern
> > > el.org/ ---
> > > 
> > >  kernel/trace/trace_kprobe.c | 42 +++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 42 insertions(+)
> > > 
> > > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > > index 23dba01831f7..0c8dd6ba650b 100644
> > > --- a/kernel/trace/trace_kprobe.c
> > > +++ b/kernel/trace/trace_kprobe.c
> > > @@ -705,6 +705,25 @@ static struct notifier_block trace_kprobe_module_nb =
> > > {> 
> > >  	.priority = 1	/* Invoked after kprobe module callback */
> > >  
> > >  };
> > > 
> > > +static int count_symbols(void *data, unsigned long unused)
> > > +{
> > > +	unsigned int *count = data;
> > > +
> > > +	(*count)++;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static unsigned int func_name_several_symbols(char *func_name)
> > 
> > If this returns boolean, please use 'bool' for return type.
> > Also, I think 'func_name_is_unique()' is more natural.
> > 
> 
> This name sounds better but it means it will check count == 1.
> I am fine with it, but please see my below comment.
> 
> > > +{
> > > +	unsigned int count;
> > > +
> > > +	count = 0;
> > > +	kallsyms_on_each_match_symbol(count_symbols, func_name, &count);
> > > +
> > > +	return count > 1;
> > > +}
> > > +
> > > 
> > >  static int __trace_kprobe_create(int argc, const char *argv[])
> > >  {
> > >  
> > >  	/*
> > > 
> > > @@ -836,6 +855,18 @@ static int __trace_kprobe_create(int argc, const char
> > > *argv[])> 
> > >  		}
> > >  	
> > >  	}
> > > 
> > > +	/*
> > > +	 * If user specifies KSYM, we check it does not correspond to several
> > > +	 * symbols.
> > > +	 * If this is the case, we return EADDRNOTAVAIL to indicate the user
> > > +	 * he/she should use ADDR rather than KSYM to remove the ambiguity.
> > > +	 */
> > > +	if (symbol && func_name_several_symbols(symbol)) {
> > 
> > Then, here  will be
> > 
> > 	if (symbol && !func_name_is_unique(symbol)) {
> > 
> 
> I am fine with the above, but it means if users gives an unknown symbol, we 
> will return EADDRNOTAVAIL instead of currently returning ENOENT.
> Is it OK?

Ah, good catch! Hm, then what about 'int number_of_same_symbols()' ?


if (symbol) {
	num = number_of_same_symbols(symbol);
	if (num > 1)
		return -EADDRNOTAVAIL;
	else if (num == 0)
		return -ENOENT;
}

Thank you,

> 
> > > +		ret = -EADDRNOTAVAIL;
> > > +
> > > +		goto error;
> > > +	}
> > > +
> > > 
> > >  	trace_probe_log_set_index(0);
> > >  	if (event) {
> > >  	
> > >  		ret = traceprobe_parse_event_name(&event, &group, gbuf,
> > > 
> > > @@ -1699,6 +1730,7 @@ static int unregister_kprobe_event(struct
> > > trace_kprobe *tk)> 
> > >  }
> > >  
> > >  #ifdef CONFIG_PERF_EVENTS
> > > 
> > > +
> > > 
> > >  /* create a trace_kprobe, but don't add it to global lists */
> > >  struct trace_event_call *
> > >  create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
> > > 
> > > @@ -1709,6 +1741,16 @@ create_local_trace_kprobe(char *func, void *addr,
> > > unsigned long offs,> 
> > >  	int ret;
> > >  	char *event;
> > > 
> > > +	/*
> > > +	 * If user specifies func, we check that function name does not
> > > +	 * correspond to several symbols.
> > > +	 * If this is the case, we return EADDRNOTAVAIL to indicate the user
> > > +	 * he/she should use addr and offs rather than func to remove the
> > > +	 * ambiguity.
> > > +	 */
> > > +	if (func && func_name_several_symbols(func))
> > 
> > Ditto.
> > 
> > Thanks!
> > 
> > > +		return ERR_PTR(-EADDRNOTAVAIL);
> > > +
> > > 
> > >  	/*
> > >  	
> > >  	 * local trace_kprobes are not added to dyn_event, so they are never
> > >  	 * searched in find_trace_kprobe(). Therefore, there is no concern of
> 
> Best regards.
> 
> 


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

^ permalink raw reply

* Re: [RFC PATCH v2 1/1] tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols
From: Francis Laniel @ 2023-08-24 14:31 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: linux-kernel, Masami Hiramatsu, linux-trace-kernel,
	Steven Rostedt
In-Reply-To: <20230824220227.01c367c1d7b6219a79cf2843@kernel.org>

Hi.

Le jeudi 24 août 2023, 15:02:27 CEST Masami Hiramatsu a écrit :
> On Thu, 24 Aug 2023 12:37:34 +0200
> 
> Francis Laniel <flaniel@linux.microsoft.com> wrote:
> > 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
> > call, because the effectively called one has no kprobes.
> > 
> > So, this commit returns EADDRNOTAVAIL when func matches several symbols.
> > This way, user needs to use addr to remove the ambiguity.
> 
> Thanks for update the patch. I have some comments there.
> 
> > Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
> > Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
> > Link:
> > https://lore.kernel.org/lkml/20230819101105.b0c104ae4494a7d1f2eea742@kern
> > el.org/ ---
> > 
> >  kernel/trace/trace_kprobe.c | 42 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 42 insertions(+)
> > 
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index 23dba01831f7..0c8dd6ba650b 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -705,6 +705,25 @@ static struct notifier_block trace_kprobe_module_nb =
> > {> 
> >  	.priority = 1	/* Invoked after kprobe module callback */
> >  
> >  };
> > 
> > +static int count_symbols(void *data, unsigned long unused)
> > +{
> > +	unsigned int *count = data;
> > +
> > +	(*count)++;
> > +
> > +	return 0;
> > +}
> > +
> > +static unsigned int func_name_several_symbols(char *func_name)
> 
> If this returns boolean, please use 'bool' for return type.
> Also, I think 'func_name_is_unique()' is more natural.
> 

This name sounds better but it means it will check count == 1.
I am fine with it, but please see my below comment.

> > +{
> > +	unsigned int count;
> > +
> > +	count = 0;
> > +	kallsyms_on_each_match_symbol(count_symbols, func_name, &count);
> > +
> > +	return count > 1;
> > +}
> > +
> > 
> >  static int __trace_kprobe_create(int argc, const char *argv[])
> >  {
> >  
> >  	/*
> > 
> > @@ -836,6 +855,18 @@ static int __trace_kprobe_create(int argc, const char
> > *argv[])> 
> >  		}
> >  	
> >  	}
> > 
> > +	/*
> > +	 * If user specifies KSYM, we check it does not correspond to several
> > +	 * symbols.
> > +	 * If this is the case, we return EADDRNOTAVAIL to indicate the user
> > +	 * he/she should use ADDR rather than KSYM to remove the ambiguity.
> > +	 */
> > +	if (symbol && func_name_several_symbols(symbol)) {
> 
> Then, here  will be
> 
> 	if (symbol && !func_name_is_unique(symbol)) {
> 

I am fine with the above, but it means if users gives an unknown symbol, we 
will return EADDRNOTAVAIL instead of currently returning ENOENT.
Is it OK?

> > +		ret = -EADDRNOTAVAIL;
> > +
> > +		goto error;
> > +	}
> > +
> > 
> >  	trace_probe_log_set_index(0);
> >  	if (event) {
> >  	
> >  		ret = traceprobe_parse_event_name(&event, &group, gbuf,
> > 
> > @@ -1699,6 +1730,7 @@ static int unregister_kprobe_event(struct
> > trace_kprobe *tk)> 
> >  }
> >  
> >  #ifdef CONFIG_PERF_EVENTS
> > 
> > +
> > 
> >  /* create a trace_kprobe, but don't add it to global lists */
> >  struct trace_event_call *
> >  create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
> > 
> > @@ -1709,6 +1741,16 @@ create_local_trace_kprobe(char *func, void *addr,
> > unsigned long offs,> 
> >  	int ret;
> >  	char *event;
> > 
> > +	/*
> > +	 * If user specifies func, we check that function name does not
> > +	 * correspond to several symbols.
> > +	 * If this is the case, we return EADDRNOTAVAIL to indicate the user
> > +	 * he/she should use addr and offs rather than func to remove the
> > +	 * ambiguity.
> > +	 */
> > +	if (func && func_name_several_symbols(func))
> 
> Ditto.
> 
> Thanks!
> 
> > +		return ERR_PTR(-EADDRNOTAVAIL);
> > +
> > 
> >  	/*
> >  	
> >  	 * local trace_kprobes are not added to dyn_event, so they are never
> >  	 * searched in find_trace_kprobe(). Therefore, there is no concern of

Best regards.



^ permalink raw reply

* Re: [RFC PATCH v2 1/1] tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols
From: Masami Hiramatsu @ 2023-08-24 13:02 UTC (permalink / raw)
  To: Francis Laniel
  Cc: linux-kernel, Masami Hiramatsu, linux-trace-kernel,
	Steven Rostedt
In-Reply-To: <20230824103734.53453-2-flaniel@linux.microsoft.com>

On Thu, 24 Aug 2023 12:37:34 +0200
Francis Laniel <flaniel@linux.microsoft.com> wrote:

> 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 call, because the
> effectively called one has no kprobes.
> 
> So, this commit returns EADDRNOTAVAIL when func matches several symbols.
> This way, user needs to use addr to remove the ambiguity.

Thanks for update the patch. I have some comments there. 

> 
> Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
> Link: https://lore.kernel.org/lkml/20230819101105.b0c104ae4494a7d1f2eea742@kernel.org/
> ---
>  kernel/trace/trace_kprobe.c | 42 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 23dba01831f7..0c8dd6ba650b 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -705,6 +705,25 @@ static struct notifier_block trace_kprobe_module_nb = {
>  	.priority = 1	/* Invoked after kprobe module callback */
>  };
>  
> +static int count_symbols(void *data, unsigned long unused)
> +{
> +	unsigned int *count = data;
> +
> +	(*count)++;
> +
> +	return 0;
> +}
> +
> +static unsigned int func_name_several_symbols(char *func_name)

If this returns boolean, please use 'bool' for return type.
Also, I think 'func_name_is_unique()' is more natural.

> +{
> +	unsigned int count;
> +
> +	count = 0;
> +	kallsyms_on_each_match_symbol(count_symbols, func_name, &count);
> +
> +	return count > 1;
> +}
> +
>  static int __trace_kprobe_create(int argc, const char *argv[])
>  {
>  	/*
> @@ -836,6 +855,18 @@ static int __trace_kprobe_create(int argc, const char *argv[])
>  		}
>  	}
>  
> +	/*
> +	 * If user specifies KSYM, we check it does not correspond to several
> +	 * symbols.
> +	 * If this is the case, we return EADDRNOTAVAIL to indicate the user
> +	 * he/she should use ADDR rather than KSYM to remove the ambiguity.
> +	 */
> +	if (symbol && func_name_several_symbols(symbol)) {

Then, here  will be 

	if (symbol && !func_name_is_unique(symbol)) {

> +		ret = -EADDRNOTAVAIL;
> +
> +		goto error;
> +	}
> +
>  	trace_probe_log_set_index(0);
>  	if (event) {
>  		ret = traceprobe_parse_event_name(&event, &group, gbuf,
> @@ -1699,6 +1730,7 @@ static int unregister_kprobe_event(struct trace_kprobe *tk)
>  }
>  
>  #ifdef CONFIG_PERF_EVENTS
> +
>  /* create a trace_kprobe, but don't add it to global lists */
>  struct trace_event_call *
>  create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
> @@ -1709,6 +1741,16 @@ create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
>  	int ret;
>  	char *event;
>  
> +	/*
> +	 * If user specifies func, we check that function name does not
> +	 * correspond to several symbols.
> +	 * If this is the case, we return EADDRNOTAVAIL to indicate the user
> +	 * he/she should use addr and offs rather than func to remove the
> +	 * ambiguity.
> +	 */
> +	if (func && func_name_several_symbols(func))

Ditto.

Thanks!

> +		return ERR_PTR(-EADDRNOTAVAIL);
> +
>  	/*
>  	 * local trace_kprobes are not added to dyn_event, so they are never
>  	 * searched in find_trace_kprobe(). Therefore, there is no concern of
> -- 
> 2.34.1
> 


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

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox