2021-10-25 03:34:12

by Tiezhu Yang

[permalink] [raw]
Subject: [PATCH 0/4] Do some changes about kprobe

This patchset is based on kprobes kernel tree:
https://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git/ for-next

Tiezhu Yang (4):
samples/kretprobes: Fix return value if register_kretprobe() failed
docs, kprobes: Remove invalid URL and add new reference
test_kprobes: Move it from kernel/ to lib/
MAINTAINERS: Add git tree and missing files for KPROBES

Documentation/trace/kprobes.rst | 2 +-
MAINTAINERS | 3 +++
kernel/Makefile | 1 -
lib/Makefile | 1 +
{kernel => lib}/test_kprobes.c | 0
samples/kprobes/kretprobe_example.c | 2 +-
6 files changed, 6 insertions(+), 3 deletions(-)
rename {kernel => lib}/test_kprobes.c (100%)

--
2.1.0


2021-10-25 03:35:27

by Tiezhu Yang

[permalink] [raw]
Subject: [PATCH 4/4] MAINTAINERS: Add git tree and missing files for KPROBES

There is no git tree for KPROBES in MAINTAINERS, it is not convinent to
rebase, lib/test_kprobes.c and samples/kprobes belong to kprobe, add them.

Signed-off-by: Tiezhu Yang <[email protected]>
---
MAINTAINERS | 3 +++
1 file changed, 3 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4372473..0e9bc60 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10506,10 +10506,13 @@ M: Anil S Keshavamurthy <[email protected]>
M: "David S. Miller" <[email protected]>
M: Masami Hiramatsu <[email protected]>
S: Maintained
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git
F: Documentation/trace/kprobes.rst
F: include/asm-generic/kprobes.h
F: include/linux/kprobes.h
F: kernel/kprobes.c
+F: lib/test_kprobes.c
+F: samples/kprobes

KS0108 LCD CONTROLLER DRIVER
M: Miguel Ojeda <[email protected]>
--
2.1.0

2021-10-25 03:37:51

by Tiezhu Yang

[permalink] [raw]
Subject: [PATCH 3/4] test_kprobes: Move it from kernel/ to lib/

Since config KPROBES_SANITY_TEST is in lib/Kconfig.debug, it is better to
let test_kprobes.c in lib/, just like other similar tests found in lib/.

Signed-off-by: Tiezhu Yang <[email protected]>
---
kernel/Makefile | 1 -
lib/Makefile | 1 +
{kernel => lib}/test_kprobes.c | 0
3 files changed, 1 insertion(+), 1 deletion(-)
rename {kernel => lib}/test_kprobes.c (100%)

diff --git a/kernel/Makefile b/kernel/Makefile
index 4df609b..9e4d33d 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -85,7 +85,6 @@ obj-$(CONFIG_PID_NS) += pid_namespace.o
obj-$(CONFIG_IKCONFIG) += configs.o
obj-$(CONFIG_IKHEADERS) += kheaders.o
obj-$(CONFIG_SMP) += stop_machine.o
-obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
obj-$(CONFIG_AUDIT) += audit.o auditfilter.o
obj-$(CONFIG_AUDITSYSCALL) += auditsc.o audit_watch.o audit_fsnotify.o audit_tree.o
obj-$(CONFIG_GCOV_KERNEL) += gcov/
diff --git a/lib/Makefile b/lib/Makefile
index 2cfd339..2c70452 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -100,6 +100,7 @@ obj-$(CONFIG_TEST_MEMINIT) += test_meminit.o
obj-$(CONFIG_TEST_LOCKUP) += test_lockup.o
obj-$(CONFIG_TEST_HMM) += test_hmm.o
obj-$(CONFIG_TEST_FREE_PAGES) += test_free_pages.o
+obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o

#
# CFLAGS for compiling floating point code inside the kernel. x86/Makefile turns
diff --git a/kernel/test_kprobes.c b/lib/test_kprobes.c
similarity index 100%
rename from kernel/test_kprobes.c
rename to lib/test_kprobes.c
--
2.1.0

2021-10-25 04:07:18

by Tiezhu Yang

[permalink] [raw]
Subject: [PATCH 1/4] samples/kretprobes: Fix return value if register_kretprobe() failed

Use the actual return value instead of always -1 if register_kretprobe()
failed.

E.g. without this patch:

# insmod samples/kprobes/kretprobe_example.ko func=no_such_func
insmod: ERROR: could not insert module samples/kprobes/kretprobe_example.ko: Operation not permitted

With this patch:

# insmod samples/kprobes/kretprobe_example.ko func=no_such_func
insmod: ERROR: could not insert module samples/kprobes/kretprobe_example.ko: Unknown symbol in module

Fixes: 804defea1c02 ("Kprobes: move kprobe examples to samples/")
Signed-off-by: Tiezhu Yang <[email protected]>
---
samples/kprobes/kretprobe_example.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/samples/kprobes/kretprobe_example.c b/samples/kprobes/kretprobe_example.c
index 5dc1bf3..228321e 100644
--- a/samples/kprobes/kretprobe_example.c
+++ b/samples/kprobes/kretprobe_example.c
@@ -86,7 +86,7 @@ static int __init kretprobe_init(void)
ret = register_kretprobe(&my_kretprobe);
if (ret < 0) {
pr_err("register_kretprobe failed, returned %d\n", ret);
- return -1;
+ return ret;
}
pr_info("Planted return probe at %s: %p\n",
my_kretprobe.kp.symbol_name, my_kretprobe.kp.addr);
--
2.1.0

2021-10-25 04:07:18

by Tiezhu Yang

[permalink] [raw]
Subject: [PATCH 2/4] docs, kprobes: Remove invalid URL and add new reference

The following reference is invalid, remove it.
https://www.ibm.com/developerworks/library/l-kprobes/index.html

Add the following new reference "An introduction to KProbes":
https://lwn.net/Articles/132196/

Signed-off-by: Tiezhu Yang <[email protected]>
---
Documentation/trace/kprobes.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/trace/kprobes.rst b/Documentation/trace/kprobes.rst
index 998149c..f318bce 100644
--- a/Documentation/trace/kprobes.rst
+++ b/Documentation/trace/kprobes.rst
@@ -784,6 +784,6 @@ References

For additional information on Kprobes, refer to the following URLs:

-- https://www.ibm.com/developerworks/library/l-kprobes/index.html
+- https://lwn.net/Articles/132196/
- https://www.kernel.org/doc/ols/2006/ols2006v2-pages-109-124.pdf

--
2.1.0

2021-10-25 06:40:44

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH 0/4] Do some changes about kprobe

Hi Tiezhu,

On Mon, 25 Oct 2021 11:30:56 +0800
Tiezhu Yang <[email protected]> wrote:

> This patchset is based on kprobes kernel tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git/ for-next

Sorry for confusion, that is not the kernel branch for kprobes. Currently
the kprobes is maintained under the Steve's tracing tree.

Anyway, some of your patch should be merged. Let me pick it.

Thank you,

>
> Tiezhu Yang (4):
> samples/kretprobes: Fix return value if register_kretprobe() failed
> docs, kprobes: Remove invalid URL and add new reference
> test_kprobes: Move it from kernel/ to lib/
> MAINTAINERS: Add git tree and missing files for KPROBES
>
> Documentation/trace/kprobes.rst | 2 +-
> MAINTAINERS | 3 +++
> kernel/Makefile | 1 -
> lib/Makefile | 1 +
> {kernel => lib}/test_kprobes.c | 0
> samples/kprobes/kretprobe_example.c | 2 +-
> 6 files changed, 6 insertions(+), 3 deletions(-)
> rename {kernel => lib}/test_kprobes.c (100%)
>
> --
> 2.1.0
>


--
Masami Hiramatsu <[email protected]>

2021-10-25 07:04:05

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH 3/4] test_kprobes: Move it from kernel/ to lib/

On Mon, 25 Oct 2021 11:30:59 +0800
Tiezhu Yang <[email protected]> wrote:

> Since config KPROBES_SANITY_TEST is in lib/Kconfig.debug, it is better to
> let test_kprobes.c in lib/, just like other similar tests found in lib/.

This is also good to me. It may be a good timing to move this under the
lib/ because there is KUnit too.

Acked-by: Masami Hiramatsu <[email protected]>

Thank you,

>
> Signed-off-by: Tiezhu Yang <[email protected]>
> ---
> kernel/Makefile | 1 -
> lib/Makefile | 1 +
> {kernel => lib}/test_kprobes.c | 0
> 3 files changed, 1 insertion(+), 1 deletion(-)
> rename {kernel => lib}/test_kprobes.c (100%)
>
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 4df609b..9e4d33d 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -85,7 +85,6 @@ obj-$(CONFIG_PID_NS) += pid_namespace.o
> obj-$(CONFIG_IKCONFIG) += configs.o
> obj-$(CONFIG_IKHEADERS) += kheaders.o
> obj-$(CONFIG_SMP) += stop_machine.o
> -obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
> obj-$(CONFIG_AUDIT) += audit.o auditfilter.o
> obj-$(CONFIG_AUDITSYSCALL) += auditsc.o audit_watch.o audit_fsnotify.o audit_tree.o
> obj-$(CONFIG_GCOV_KERNEL) += gcov/
> diff --git a/lib/Makefile b/lib/Makefile
> index 2cfd339..2c70452 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -100,6 +100,7 @@ obj-$(CONFIG_TEST_MEMINIT) += test_meminit.o
> obj-$(CONFIG_TEST_LOCKUP) += test_lockup.o
> obj-$(CONFIG_TEST_HMM) += test_hmm.o
> obj-$(CONFIG_TEST_FREE_PAGES) += test_free_pages.o
> +obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
>
> #
> # CFLAGS for compiling floating point code inside the kernel. x86/Makefile turns
> diff --git a/kernel/test_kprobes.c b/lib/test_kprobes.c
> similarity index 100%
> rename from kernel/test_kprobes.c
> rename to lib/test_kprobes.c
> --
> 2.1.0
>


--
Masami Hiramatsu <[email protected]>

2021-10-25 07:04:38

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH 4/4] MAINTAINERS: Add git tree and missing files for KPROBES

On Mon, 25 Oct 2021 11:31:00 +0800
Tiezhu Yang <[email protected]> wrote:

> There is no git tree for KPROBES in MAINTAINERS, it is not convinent to
> rebase, lib/test_kprobes.c and samples/kprobes belong to kprobe, add them.
>
> Signed-off-by: Tiezhu Yang <[email protected]>
> ---
> MAINTAINERS | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4372473..0e9bc60 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10506,10 +10506,13 @@ M: Anil S Keshavamurthy <[email protected]>
> M: "David S. Miller" <[email protected]>
> M: Masami Hiramatsu <[email protected]>
> S: Maintained
> +T: git git://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git

NACK. As I said, this branch was prepared when I discussed with tip maintainer
on kretprobe stackfix series but now it has been maintained on Steve's tracing
tree.

Steve, should we put your tree here?

Anyway, I will pick your [1/4]-[3/4].

Thank you,

> F: Documentation/trace/kprobes.rst
> F: include/asm-generic/kprobes.h
> F: include/linux/kprobes.h
> F: kernel/kprobes.c
> +F: lib/test_kprobes.c
> +F: samples/kprobes
>
> KS0108 LCD CONTROLLER DRIVER
> M: Miguel Ojeda <[email protected]>
> --
> 2.1.0
>


--
Masami Hiramatsu <[email protected]>

2021-10-25 07:54:12

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH 1/4] samples/kretprobes: Fix return value if register_kretprobe() failed

On Mon, 25 Oct 2021 11:30:57 +0800
Tiezhu Yang <[email protected]> wrote:

> Use the actual return value instead of always -1 if register_kretprobe()
> failed.
>
> E.g. without this patch:
>
> # insmod samples/kprobes/kretprobe_example.ko func=no_such_func
> insmod: ERROR: could not insert module samples/kprobes/kretprobe_example.ko: Operation not permitted
>
> With this patch:
>
> # insmod samples/kprobes/kretprobe_example.ko func=no_such_func
> insmod: ERROR: could not insert module samples/kprobes/kretprobe_example.ko: Unknown symbol in module
>
> Fixes: 804defea1c02 ("Kprobes: move kprobe examples to samples/")
> Signed-off-by: Tiezhu Yang <[email protected]>

This looks good to me.

Acked-by: Masami Hiramatsu <[email protected]>

Thanks!

> ---
> samples/kprobes/kretprobe_example.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/samples/kprobes/kretprobe_example.c b/samples/kprobes/kretprobe_example.c
> index 5dc1bf3..228321e 100644
> --- a/samples/kprobes/kretprobe_example.c
> +++ b/samples/kprobes/kretprobe_example.c
> @@ -86,7 +86,7 @@ static int __init kretprobe_init(void)
> ret = register_kretprobe(&my_kretprobe);
> if (ret < 0) {
> pr_err("register_kretprobe failed, returned %d\n", ret);
> - return -1;
> + return ret;
> }
> pr_info("Planted return probe at %s: %p\n",
> my_kretprobe.kp.symbol_name, my_kretprobe.kp.addr);
> --
> 2.1.0
>


--
Masami Hiramatsu <[email protected]>

2021-10-25 07:56:38

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH 2/4] docs, kprobes: Remove invalid URL and add new reference

On Mon, 25 Oct 2021 11:30:58 +0800
Tiezhu Yang <[email protected]> wrote:

> The following reference is invalid, remove it.
> https://www.ibm.com/developerworks/library/l-kprobes/index.html
>
> Add the following new reference "An introduction to KProbes":
> https://lwn.net/Articles/132196/

Looks good to me.

Acked-by: Masami Hiramatsu <[email protected]>

Thanks!

>
> Signed-off-by: Tiezhu Yang <[email protected]>
> ---
> Documentation/trace/kprobes.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/trace/kprobes.rst b/Documentation/trace/kprobes.rst
> index 998149c..f318bce 100644
> --- a/Documentation/trace/kprobes.rst
> +++ b/Documentation/trace/kprobes.rst
> @@ -784,6 +784,6 @@ References
>
> For additional information on Kprobes, refer to the following URLs:
>
> -- https://www.ibm.com/developerworks/library/l-kprobes/index.html
> +- https://lwn.net/Articles/132196/
> - https://www.kernel.org/doc/ols/2006/ols2006v2-pages-109-124.pdf
>
> --
> 2.1.0
>


--
Masami Hiramatsu <[email protected]>

2021-10-25 11:20:24

by Tiezhu Yang

[permalink] [raw]
Subject: Re: [PATCH 4/4] MAINTAINERS: Add git tree and missing files for KPROBES

On 10/25/2021 02:38 PM, Masami Hiramatsu wrote:
> On Mon, 25 Oct 2021 11:31:00 +0800
> Tiezhu Yang <[email protected]> wrote:
>
>> There is no git tree for KPROBES in MAINTAINERS, it is not convinent to
>> rebase, lib/test_kprobes.c and samples/kprobes belong to kprobe, add them.
>>
>> Signed-off-by: Tiezhu Yang <[email protected]>
>> ---
>> MAINTAINERS | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 4372473..0e9bc60 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -10506,10 +10506,13 @@ M: Anil S Keshavamurthy <[email protected]>
>> M: "David S. Miller" <[email protected]>
>> M: Masami Hiramatsu <[email protected]>
>> S: Maintained
>> +T: git git://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git
> NACK. As I said, this branch was prepared when I discussed with tip maintainer
> on kretprobe stackfix series but now it has been maintained on Steve's tracing
> tree.
>
> Steve, should we put your tree here?

If yes, should I update patch #4 and then send a v2 version of this
patch set?

@@ -10505,11 +10505,16 @@ M: Naveen N. Rao <[email protected]>
M: Anil S Keshavamurthy <[email protected]>
M: "David S. Miller" <[email protected]>
M: Masami Hiramatsu <[email protected]>
+L: [email protected]
S: Maintained
+Q: https://patchwork.kernel.org/project/linux-trace-devel/list/
+T: git
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
F: Documentation/trace/kprobes.rst
F: include/asm-generic/kprobes.h
F: include/linux/kprobes.h
F: kernel/kprobes.c
+F: lib/test_kprobes.c
+F: samples/kprobes

By the way, it seems that we should also update the TRACING git tree [1]?
If yes, should I send the following change as a new patch #5 in the v2
version
or do the following change in the above patch #4?

@@ -19065,8 +19070,10 @@ F: drivers/char/tpm/
TRACING
M: Steven Rostedt <[email protected]>
M: Ingo Molnar <[email protected]>
+L: [email protected]
S: Maintained
-T: git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
perf/core
+Q: https://patchwork.kernel.org/project/linux-trace-devel/list/
+T: git
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
F: Documentation/trace/ftrace.rst
F: arch/*/*/*/ftrace.h
F: arch/*/kernel/ftrace.c

[1]
https://lore.kernel.org/lkml/[email protected]/

Thanks,
Tiezhu

>
> Anyway, I will pick your [1/4]-[3/4].
>
> Thank you,
>
>> F: Documentation/trace/kprobes.rst
>> F: include/asm-generic/kprobes.h
>> F: include/linux/kprobes.h
>> F: kernel/kprobes.c
>> +F: lib/test_kprobes.c
>> +F: samples/kprobes
>>
>> KS0108 LCD CONTROLLER DRIVER
>> M: Miguel Ojeda <[email protected]>
>> --
>> 2.1.0
>>
>

2021-10-25 14:52:25

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 4/4] MAINTAINERS: Add git tree and missing files for KPROBES

On Mon, 25 Oct 2021 16:31:07 +0800
Tiezhu Yang <[email protected]> wrote:

> On 10/25/2021 02:38 PM, Masami Hiramatsu wrote:
> > On Mon, 25 Oct 2021 11:31:00 +0800
> > Tiezhu Yang <[email protected]> wrote:
> >
> >> There is no git tree for KPROBES in MAINTAINERS, it is not convinent to
> >> rebase, lib/test_kprobes.c and samples/kprobes belong to kprobe, add them.
> >>
> >> Signed-off-by: Tiezhu Yang <[email protected]>
> >> ---
> >> MAINTAINERS | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index 4372473..0e9bc60 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -10506,10 +10506,13 @@ M: Anil S Keshavamurthy <[email protected]>
> >> M: "David S. Miller" <[email protected]>
> >> M: Masami Hiramatsu <[email protected]>
> >> S: Maintained
> >> +T: git git://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git
> > NACK. As I said, this branch was prepared when I discussed with tip maintainer
> > on kretprobe stackfix series but now it has been maintained on Steve's tracing
> > tree.
> >
> > Steve, should we put your tree here?
>

I'm fine if kprobes goes through my tree.

> If yes, should I update patch #4 and then send a v2 version of this
> patch set?
>
> @@ -10505,11 +10505,16 @@ M: Naveen N. Rao <[email protected]>
> M: Anil S Keshavamurthy <[email protected]>
> M: "David S. Miller" <[email protected]>
> M: Masami Hiramatsu <[email protected]>
> +L: [email protected]

Please do not add the above mailing list. That's more for tracing tools
like trace-cmd, kernelshark and the libtracefs/libtraceevent libraries.
Only API changes (additions) should go to that list. Not internal updates.

> S: Maintained
> +Q: https://patchwork.kernel.org/project/linux-trace-devel/list/

And this too is for the tracing tools, not the kernel. The patches status
on that patchwork do not get updated by changes to the kernel. Only changes
to the libraries and tool git trees will update those patches.

> +T: git
> git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
> F: Documentation/trace/kprobes.rst
> F: include/asm-generic/kprobes.h
> F: include/linux/kprobes.h
> F: kernel/kprobes.c
> +F: lib/test_kprobes.c
> +F: samples/kprobes
>
> By the way, it seems that we should also update the TRACING git tree [1]?
> If yes, should I send the following change as a new patch #5 in the v2
> version
> or do the following change in the above patch #4?
>
> @@ -19065,8 +19070,10 @@ F: drivers/char/tpm/
> TRACING
> M: Steven Rostedt <[email protected]>
> M: Ingo Molnar <[email protected]>
> +L: [email protected]

Again, don't add that list.

> S: Maintained
> -T: git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
> perf/core
> +Q: https://patchwork.kernel.org/project/linux-trace-devel/list/

Nor the patchwork.

> +T: git
> git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
> F: Documentation/trace/ftrace.rst
> F: arch/*/*/*/ftrace.h
> F: arch/*/kernel/ftrace.c
>

-- Steve