From 6f6471ce0cd7c48dcb9fba85acdbf0b851093115 Mon Sep 17 00:00:00 2001 From: Eden Federman Date: Mon, 8 May 2023 07:51:38 +0300 Subject: [PATCH 1/6] fix gRPC instrumentation on recent kernels --- include/go_types.h | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/include/go_types.h b/include/go_types.h index 477ef6f3d..43c1cced0 100644 --- a/include/go_types.h +++ b/include/go_types.h @@ -16,6 +16,7 @@ #include "bpf_helpers.h" #define MAX_REALLOCATION 400 +#define MAX_DATA_SIZE 400 struct go_string { @@ -72,6 +73,11 @@ static __always_inline void append_item_to_slice(struct go_slice *slice, void *n else { // No room on current array - copy to new one of size item_size * (len + 1) + if (slice->len > MAX_DATA_SIZE || slice->len < 1) + { + return; + } + s32 alloc_size = item_size * slice->len; s32 bounded_alloc_size = alloc_size > MAX_REALLOCATION ? MAX_REALLOCATION : (alloc_size < 1 ? 1 : alloc_size); @@ -86,7 +92,15 @@ static __always_inline void append_item_to_slice(struct go_slice *slice, void *n // Append to buffer bpf_probe_read_user(map_buff, bounded_alloc_size, slice->array); bpf_probe_read(map_buff + bounded_alloc_size, item_size, new_item); - void *new_array = write_target_data(map_buff, bounded_alloc_size + item_size); + + // Copy buffer to userspace + u32 new_array_size = bounded_alloc_size + item_size; + if (new_array_size > MAX_DATA_SIZE || new_array_size < 1) + { + return; + } + + void *new_array = write_target_data(map_buff, new_array_size); // Update array slice->array = new_array; @@ -100,4 +114,4 @@ static __always_inline void append_item_to_slice(struct go_slice *slice, void *n // Update len slice->len++; long success = bpf_probe_write_user(slice_user_ptr->len, &slice->len, sizeof(slice->len)); -} +} \ No newline at end of file From b2be27e71a637f96f805bcd6e75208b456ab4f4a Mon Sep 17 00:00:00 2001 From: Eden Federman Date: Mon, 8 May 2023 19:41:51 +0300 Subject: [PATCH 2/6] bound buffer size --- include/alloc.h | 18 ++++++++++++++++++ .../bpf/google/golang/org/grpc/server/probe.go | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/include/alloc.h b/include/alloc.h index c494ed4d1..a2314c093 100644 --- a/include/alloc.h +++ b/include/alloc.h @@ -15,6 +15,7 @@ #include "bpf_helpers.h" #define MAX_ENTRIES 50 +#define MAX_BUFFER_SIZE 1024 // Injected in init volatile const u32 total_cpus; @@ -65,6 +66,22 @@ static __always_inline u64 get_area_end(u64 start) } } +static __always_inline s32 bound_number(s32 num, s32 min, s32 max) +{ + if (num < min) + { + return min; + } + else if (num > max) + { + return max; + } + else + { + return num; + } +} + static __always_inline void *write_target_data(void *data, s32 size) { if (!data || data == NULL) @@ -83,6 +100,7 @@ static __always_inline void *write_target_data(void *data, s32 size) } void *target = (void *)start; + size = bound_number(size, 1, MAX_BUFFER_SIZE); long success = bpf_probe_write_user(target, data, size); if (success == 0) { diff --git a/pkg/instrumentors/bpf/google/golang/org/grpc/server/probe.go b/pkg/instrumentors/bpf/google/golang/org/grpc/server/probe.go index f1c936b79..50df70fb4 100644 --- a/pkg/instrumentors/bpf/google/golang/org/grpc/server/probe.go +++ b/pkg/instrumentors/bpf/google/golang/org/grpc/server/probe.go @@ -129,7 +129,7 @@ func (g *Instrumentor) Load(ctx *context.InstrumentorContext) error { } up, err := ctx.Executable.Uprobe("", g.bpfObjects.UprobeServerHandleStream, &link.UprobeOptions{ - Offset: offset, + Address: offset, }) if err != nil { return err From 569f50baf573c143a12d35061e50e0716348c12f Mon Sep 17 00:00:00 2001 From: Eden Federman Date: Mon, 8 May 2023 19:55:17 +0300 Subject: [PATCH 3/6] add CHANGELOG entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a84fdaa82..aa0bf6820 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ OpenTelemetry Go Automatic Instrumentation adheres to [Semantic Versioning](http ## [Unreleased] +### Changed + +- Fix gRPC instrumentation on recent kernels ([#150](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/150)) + ## [v0.2.0-alpha] - 2023-05-03 ### Added From b2392d4ad141a38637e59f90a65b931d68ed28a1 Mon Sep 17 00:00:00 2001 From: Eden Federman Date: Mon, 8 May 2023 22:27:27 -0700 Subject: [PATCH 4/6] Update CHANGELOG.md Co-authored-by: Tyler Yahn --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa0bf6820..f00fe12a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ OpenTelemetry Go Automatic Instrumentation adheres to [Semantic Versioning](http ## [Unreleased] -### Changed +### Fixed - Fix gRPC instrumentation on recent kernels ([#150](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/150)) From 21b2b1017c3a467b65ed56c78aa6cde602d21426 Mon Sep 17 00:00:00 2001 From: Eden Federman Date: Mon, 8 May 2023 22:27:40 -0700 Subject: [PATCH 5/6] Update CHANGELOG.md Co-authored-by: Tyler Yahn --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f00fe12a3..79de3d4ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ OpenTelemetry Go Automatic Instrumentation adheres to [Semantic Versioning](http ### Fixed -- Fix gRPC instrumentation on recent kernels ([#150](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/150)) +- Fix gRPC instrumentation memory access issue on newer kernels. ([#150](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/150)) ## [v0.2.0-alpha] - 2023-05-03 From 416c8bd46ce063e1fbf46c5dab933dc8b4267e56 Mon Sep 17 00:00:00 2001 From: Eden Federman Date: Tue, 9 May 2023 16:26:12 +0300 Subject: [PATCH 6/6] code review comments --- include/alloc.h | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/include/alloc.h b/include/alloc.h index a2314c093..896822d1b 100644 --- a/include/alloc.h +++ b/include/alloc.h @@ -16,6 +16,7 @@ #define MAX_ENTRIES 50 #define MAX_BUFFER_SIZE 1024 +#define MIN_BUFFER_SIZE 1 // Injected in init volatile const u32 total_cpus; @@ -76,10 +77,7 @@ static __always_inline s32 bound_number(s32 num, s32 min, s32 max) { return max; } - else - { - return num; - } + return num; } static __always_inline void *write_target_data(void *data, s32 size) @@ -100,7 +98,7 @@ static __always_inline void *write_target_data(void *data, s32 size) } void *target = (void *)start; - size = bound_number(size, 1, MAX_BUFFER_SIZE); + size = bound_number(size, MIN_BUFFER_SIZE, MAX_BUFFER_SIZE); long success = bpf_probe_write_user(target, data, size); if (success == 0) {