From 7cf8e2d7bfd4aa95e7d27978176ebda0460a0712 Mon Sep 17 00:00:00 2001 From: Jerome Marchand Date: Jul 10 2024 12:02:45 +0000 Subject: Fix SAST vulnerabilities Resolves: RHEL-40624 Signed-off-by: Jerome Marchand --- diff --git a/libtraceevent-Close-shared-object-in-the-error-path-.patch b/libtraceevent-Close-shared-object-in-the-error-path-.patch new file mode 100644 index 0000000..6155a65 --- /dev/null +++ b/libtraceevent-Close-shared-object-in-the-error-path-.patch @@ -0,0 +1,61 @@ +From 34ece90e09559089da0bfec1a1a03396fd507178 Mon Sep 17 00:00:00 2001 +From: Jerome Marchand +Date: Fri, 7 Jun 2024 18:05:39 +0200 +Subject: [PATCH 3/7] libtraceevent: Close shared object in the error path of + load_plugin() + +The handle returned by dlopen() isn't close if an error occurs +afterward. Call dlclose() in the error path. + +Fixes a RESOURCE_LEAK error (CWE-772) + +Link: https://lore.kernel.org/linux-trace-devel/20240607160542.46152-2-jmarchan@redhat.com + +Fixes: 7e95ebdbbc3a9 ("tools lib traceevent: Add plugin support") +Signed-off-by: Jerome Marchand +Signed-off-by: Steven Rostedt (Google) +--- + src/event-plugin.c | 8 +++++--- + 1 file changed, 5 insertions(+), 3 deletions(-) + +diff --git a/src/event-plugin.c b/src/event-plugin.c +index f42243f..7f94107 100644 +--- a/src/event-plugin.c ++++ b/src/event-plugin.c +@@ -474,7 +474,7 @@ load_plugin(struct tep_handle *tep, const char *path, + while (options->name) { + ret = update_option(alias, options); + if (ret < 0) +- goto out_free; ++ goto out_close; + options++; + } + } +@@ -483,13 +483,13 @@ load_plugin(struct tep_handle *tep, const char *path, + if (!func) { + tep_warning("could not find func '%s' in plugin '%s'\n%s\n", + TEP_PLUGIN_LOADER_NAME, plugin, dlerror()); +- goto out_free; ++ goto out_close; + } + + list = malloc(sizeof(*list)); + if (!list) { + tep_warning("could not allocate plugin memory\n"); +- goto out_free; ++ goto out_close; + } + + list->next = *plugin_list; +@@ -501,6 +501,8 @@ load_plugin(struct tep_handle *tep, const char *path, + func(tep); + return; + ++out_close: ++ dlclose(handle); + out_free: + free(plugin); + } +-- +2.45.2 + diff --git a/libtraceevent-Do-not-return-a-local-stack-pointer-in.patch b/libtraceevent-Do-not-return-a-local-stack-pointer-in.patch new file mode 100644 index 0000000..5d29928 --- /dev/null +++ b/libtraceevent-Do-not-return-a-local-stack-pointer-in.patch @@ -0,0 +1,44 @@ +From 021da909bcbf657ceccbc1bcfa34b3d5c029be80 Mon Sep 17 00:00:00 2001 +From: "Steven Rostedt (Google)" +Date: Fri, 14 Jun 2024 15:54:56 -0400 +Subject: [PATCH 7/7] libtraceevent: Do not return a local stack pointer in + get_field_str() + +Jerome Marchand sent a patch with the description of: + + The function get_field_str() can return a pointer to string on the + stack. Replace it by a global variable. + + Fixes a RETURN_LOCAL error (CWE-562) + +But made hex a global variable. Having a generic name "hex" as a global +variable in a library will cause a lot of issues. Just make it a static +variable, and then it can be used outside the function. + +Link: https://lore.kernel.org/linux-trace-devel/20240607160542.46152-5-jmarchan@redhat.com/ +Link: https://lore.kernel.org/linux-trace-devel/20240614155456.092944eb@rorschach.local.home + +Fixes: dee43d8067350 ("tools lib traceevent: Let filtering numbers by string use function names") +Reported-by: "Jerome Marchand" +Signed-off-by: Steven Rostedt (Google) +--- + src/parse-filter.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/parse-filter.c b/src/parse-filter.c +index e448ee2..75b84a0 100644 +--- a/src/parse-filter.c ++++ b/src/parse-filter.c +@@ -1704,8 +1704,8 @@ static const char *get_field_str(struct tep_filter_arg *arg, struct tep_record * + struct tep_handle *tep; + unsigned long long addr; + const char *val = NULL; ++ static char hex[64]; + unsigned int size; +- char hex[64]; + + /* If the field is not a string convert it */ + if (arg->str.field->flags & TEP_FIELD_IS_STRING) { +-- +2.45.2 + diff --git a/libtraceevent-Fix-event-parse-memory-leak-in-process.patch b/libtraceevent-Fix-event-parse-memory-leak-in-process.patch new file mode 100644 index 0000000..dc5266b --- /dev/null +++ b/libtraceevent-Fix-event-parse-memory-leak-in-process.patch @@ -0,0 +1,99 @@ +From 76a0eb8d5a20c69120a5f8b4c12f4da0cdc15bb5 Mon Sep 17 00:00:00 2001 +From: Ian Rogers +Date: Tue, 30 Apr 2024 00:39:08 -0700 +Subject: [PATCH 1/7] libtraceevent: Fix event-parse memory leak in + process_cond + +Leak sanitizer was reporting a stack trace with perf: +``` +$ perf stat -e 'kvm:kvm_inj_exception' true + + Performance counter stats for 'true': + + 0 kvm:kvm_inj_exception + + 0.001701473 seconds time elapsed + + 0.000000000 seconds user + 0.001865000 seconds sys + +================================================================= +==1705137==ERROR: LeakSanitizer: detected memory leaks + +Direct leak of 2 byte(s) in 1 object(s) allocated from: + #0 0x7f413ee80778 in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:454 + #1 0x7f413ecb7b66 in __read_token libtraceevent/src/event-parse.c:1274 + #2 0x7f413ecb85bb in read_token libtraceevent/src/event-parse.c:1432 + #3 0x7f413ecbeaaa in process_entry libtraceevent/src/event-parse.c:2554 + #4 0x7f413ecc54ae in process_arg_token libtraceevent/src/event-parse.c:3698 + #5 0x7f413ecbb52e in process_arg libtraceevent/src/event-parse.c:2017 + #6 0x7f413ecbd05a in process_op libtraceevent/src/event-parse.c:2357 + #7 0x7f413ecc5a56 in process_arg_token libtraceevent/src/event-parse.c:3752 + #8 0x7f413ecbb52e in process_arg libtraceevent/src/event-parse.c:2017 + #9 0x7f413ecc5dd6 in event_read_print_args libtraceevent/src/event-parse.c:3791 + #10 0x7f413ecc6511 in event_read_print libtraceevent/src/event-parse.c:3879 + #11 0x7f413ecda16c in parse_format libtraceevent/src/event-parse.c:7808 + #12 0x7f413ecda667 in __parse_event libtraceevent/src/event-parse.c:7866 + #13 0x7f413ecda71b in tep_parse_format libtraceevent/src/event-parse.c:7908 + #14 0x561672439029 in tp_format util/trace-event.c:94 + #15 0x561672439141 in trace_event__tp_format util/trace-event.c:109 + #16 0x56167230a429 in evsel__newtp_idx util/evsel.c:472 + #17 0x561672329f99 in add_tracepoint util/parse-events.c:552 + #18 0x56167232a5b4 in add_tracepoint_event util/parse-events.c:627 + #19 0x56167232ebf2 in parse_events_add_tracepoint util/parse-events.c:1313 + #20 0x561672411e0e in parse_events_parse util/parse-events.y:500 + #21 0x561672332409 in parse_events__scanner util/parse-events.c:1878 + #22 0x561672333cd4 in __parse_events util/parse-events.c:2146 + #23 0x561672334e74 in parse_events_option util/parse-events.c:2349 + #24 0x56167269ec23 in get_value tools/lib/subcmd/parse-options.c:251 + #25 0x56167269fe65 in parse_short_opt tools/lib/subcmd/parse-options.c:351 + #26 0x5616726a0e4d in parse_options_step tools/lib/subcmd/parse-options.c:539 + #27 0x5616726a1d86 in parse_options_subcommand tools/lib/subcmd/parse-options.c:654 + #28 0x5616720e6ad2 in cmd_stat tools/perf/builtin-stat.c:2531 + #29 0x5616722b0f5d in run_builtin tools/perf/perf.c:350 +$ cat /sys/kernel/tracing/events/kvm/kvm_inj_exception/format +name: kvm_inj_exception +ID: 1956 +format: + field:unsigned short common_type; offset:0; size:2; signed:0; + field:unsigned char common_flags; offset:2; size:1; signed:0; + field:unsigned char common_preempt_count; offset:3; size:1; signed:0; + field:int common_pid; offset:4; size:4; signed:1; + + field:u8 exception; offset:8; size:1; signed:0; + field:u8 has_error; offset:9; size:1; signed:0; + field:u32 error_code; offset:12; size:4; signed:0; + field:bool reinjected; offset:16; size:1; signed:0; + +print fmt: "%s%s%s%s%s", __print_symbolic(REC->exception, { 0, "#" "DE" }, { 1, "#" "DB" }, { 3, "#" "BP" }, { 4, "#" "OF" }, { 5, "#" "BR" }, { 6, "#" "UD" }, { 7, "#" "NM" }, { 8, "#" "DF" }, { 10, "#" "TS" }, { 11, "#" "NP" }, { 12, "#" "SS" }, { 13, "#" "GP" }, { 14, "#" "PF" }, { 16, "#" "MF" }, { 17, "#" "AC" }, { 18, "#" "MC" }), !REC->has_error ? "" : " (", !REC->has_error ? "" : __print_symbolic(REC->error_code, { }), !REC->has_error ? "" : ")", REC->reinjected ? " [reinjected]" : "" +``` + +The issue appears to be that when process_cond returns an error, +callers clear the variable holding the string but the string was never +freed. This change adds the free when process_cond returns +TEP_EVENT_ERROR. + +Link: https://lore.kernel.org/linux-trace-devel/20240430073908.1706482-1-irogers@google.com + +Signed-off-by: Ian Rogers +Signed-off-by: Steven Rostedt (Google) +--- + src/event-parse.c | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/src/event-parse.c b/src/event-parse.c +index 61b0966..2c38fe5 100644 +--- a/src/event-parse.c ++++ b/src/event-parse.c +@@ -2373,6 +2373,8 @@ process_op(struct tep_event *event, struct tep_print_arg *arg, char **tok) + + /* it will set arg->op.right */ + type = process_cond(event, arg, tok); ++ if (type == TEP_EVENT_ERROR) ++ free(token); + + } else if (strcmp(token, ">>") == 0 || + strcmp(token, "<<") == 0 || +-- +2.45.2 + diff --git a/libtraceevent-Have-unit-test-fail-when-any-tests-fai.patch b/libtraceevent-Have-unit-test-fail-when-any-tests-fai.patch new file mode 100644 index 0000000..e52a2c6 --- /dev/null +++ b/libtraceevent-Have-unit-test-fail-when-any-tests-fai.patch @@ -0,0 +1,41 @@ +From 340e2e673f8951f049e9250621e3a4d4e84f10dc Mon Sep 17 00:00:00 2001 +From: "Steven Rostedt (Google)" +Date: Fri, 14 Jun 2024 15:34:21 -0400 +Subject: [PATCH 6/7] libtraceevent: Have unit test fail when any tests fail + +If any of the tests of the unit test fails, make sure it returns non-zero to +allow tools that use this know that a test failed. + +Link: https://lore.kernel.org/linux-trace-devel/20240329135331.784707-1-paul.mars@canonical.com/ +Link: https://lore.kernel.org/linux-trace-devel/20240614153421.2c934dc1@rorschach.local.home + +Reported-by: Paul Mars +Signed-off-by: Steven Rostedt (Google) +--- + utest/trace-utest.c | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +diff --git a/utest/trace-utest.c b/utest/trace-utest.c +index a26e42e..7c4b9b6 100644 +--- a/utest/trace-utest.c ++++ b/utest/trace-utest.c +@@ -37,6 +37,7 @@ int main(int argc, char **argv) + { + CU_BasicRunMode verbose = CU_BRM_VERBOSE; + enum unit_tests tests = RUN_NONE; ++ int failed_tests; + + for (;;) { + int c; +@@ -82,6 +83,7 @@ int main(int argc, char **argv) + + CU_basic_set_mode(verbose); + CU_basic_run_tests(); ++ failed_tests = CU_get_number_of_tests_failed(); + CU_cleanup_registry(); +- return 0; ++ return failed_tests != 0; + } +-- +2.45.2 + diff --git a/libtraceevent-Prevent-a-memory-leak-in-process_field.patch b/libtraceevent-Prevent-a-memory-leak-in-process_field.patch new file mode 100644 index 0000000..f584c6e --- /dev/null +++ b/libtraceevent-Prevent-a-memory-leak-in-process_field.patch @@ -0,0 +1,35 @@ +From 03551ebce2a745127a9b6cf3765381c05621b27a Mon Sep 17 00:00:00 2001 +From: Jerome Marchand +Date: Fri, 7 Jun 2024 18:05:40 +0200 +Subject: [PATCH 4/7] libtraceevent: Prevent a memory leak in process_fields() + +One of the error paths after the field was allocated goes to the wrong +label. Go to out_free_field if the allocation of arg fails. + +Fixes a RESOURCE_LEAK error (CWE-772) + +Link: https://lore.kernel.org/linux-trace-devel/20240607160542.46152-3-jmarchan@redhat.com + +Fixes: b17b75e511722 ("tools lib traceevent: Handle alloc_arg failure") +Signed-off-by: Jerome Marchand +Signed-off-by: Steven Rostedt (Google) +--- + src/event-parse.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/event-parse.c b/src/event-parse.c +index b625621..9f0522c 100644 +--- a/src/event-parse.c ++++ b/src/event-parse.c +@@ -2963,7 +2963,7 @@ process_fields(struct tep_event *event, struct tep_print_flag_sym **list, char * + free_arg(arg); + arg = alloc_arg(); + if (!arg) +- goto out_free; ++ goto out_free_field; + + free_token(token); + type = process_arg(event, arg, &token); +-- +2.45.2 + diff --git a/libtraceevent-prevent-a-memory-leak-in-tep_plugin_ad.patch b/libtraceevent-prevent-a-memory-leak-in-tep_plugin_ad.patch new file mode 100644 index 0000000..4304405 --- /dev/null +++ b/libtraceevent-prevent-a-memory-leak-in-tep_plugin_ad.patch @@ -0,0 +1,36 @@ +From c84155f7dfedeb0e0c0c00f5fae7bad67f494de7 Mon Sep 17 00:00:00 2001 +From: Jerome Marchand +Date: Fri, 7 Jun 2024 18:05:41 +0200 +Subject: [PATCH 5/7] libtraceevent: prevent a memory leak in + tep_plugin_add_option() + +If parse_option_name() fails, plugin, which now points to the previous +value of option_str isn't freed. Go to out_free if that happens. + +Fixes a RESOURCE_LEAK error (CWE-772) + +Link: https://lore.kernel.org/linux-trace-devel/20240607160542.46152-4-jmarchan@redhat.com + +Fixes: 442ac241bef96 ("libtraceevent: Handle strdup() error in parse_option_name()") +Signed-off-by: Jerome Marchand +Signed-off-by: Steven Rostedt (Google) +--- + src/event-plugin.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/event-plugin.c b/src/event-plugin.c +index 7f94107..c944204 100644 +--- a/src/event-plugin.c ++++ b/src/event-plugin.c +@@ -327,7 +327,7 @@ int tep_plugin_add_option(const char *name, const char *val) + return -ENOMEM; + + if (parse_option_name(&option_str, &plugin) < 0) +- return -ENOMEM; ++ goto out_free; + + /* If the option exists, update the val */ + for (op = trace_plugin_options; op; op = op->next) { +-- +2.45.2 + diff --git a/libtraceevent.spec b/libtraceevent.spec index a195aba..cf5ad5f 100644 --- a/libtraceevent.spec +++ b/libtraceevent.spec @@ -5,7 +5,7 @@ Name: libtraceevent Version: 1.8.2 -Release: 4%{?dist} +Release: 5%{?dist} License: LGPL-2.1-only AND LGPL-2.1-or-later AND GPL-2.0-only AND GPL-2.0-or-later Summary: Library to parse raw trace event formats @@ -16,6 +16,13 @@ URL: https://git.kernel.org/pub/scm/libs/libtrace/libtraceevent.git/ # git archive --prefix=libtraceevent-%%{version}/ -o libtraceevent-%%{version}.tar.gz %%{git_commit} #Source0: libtraceevent-%%{version}.tar.gz Source0: https://git.kernel.org/pub/scm/libs/libtrace/libtraceevent.git/snapshot/libtraceevent-%{version}.tar.gz +Patch0: libtraceevent-Fix-event-parse-memory-leak-in-process.patch +Patch1: libtraceevent-Close-shared-object-in-the-error-path-.patch +Patch2: libtraceevent-Prevent-a-memory-leak-in-process_field.patch +Patch3: libtraceevent-prevent-a-memory-leak-in-tep_plugin_ad.patch +Patch4: libtraceevent-Have-unit-test-fail-when-any-tests-fai.patch +Patch5: libtraceevent-Do-not-return-a-local-stack-pointer-in.patch + BuildRequires: gcc BuildRequires: xmlto BuildRequires: asciidoc @@ -34,7 +41,7 @@ Requires: %{name}%{_isa} = %{version}-%{release} Development headers of %{name}-libs %prep -%setup -q +%autosetup -p1 %build MANPAGE_DOCBOOK_XSL=`rpm -ql docbook-style-xsl | grep manpages/docbook.xsl` @@ -63,6 +70,9 @@ rm -rf %{buildroot}/%{_libdir}/libtraceevent.a %{_libdir}/pkgconfig/libtraceevent.pc %changelog +* Wed Jul 10 2024 Jerome Marchand - 1.8.2-5 +- Fix SAST vulnerabilities (RHEL-40624) + * Mon Jun 24 2024 Troy Dawson - 1.8.2-4 - Bump release for June 2024 mass rebuild