Skip to content

Commit bdd7ca0

Browse files
sywhangsywhang
authored
Enforce 64KB event payload size limit on EventPipe (#50600)
* allow >100KB events to be written * prevent events > 64KB from being written * code review feedback * increment seq num * fix build * fix build on clang * fix mono build
1 parent 8d6cd81 commit bdd7ca0

File tree

4 files changed

+130
-9
lines changed

4 files changed

+130
-9
lines changed

‎src/native/eventpipe/ep-buffer-manager.c‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,7 @@ ep_buffer_manager_alloc (
787787

788788
instance->session = session;
789789
instance->size_of_all_buffers = 0;
790+
instance->num_oversized_events_dropped = 0;
790791

791792
#ifdef EP_CHECKED_BUILD
792793
instance->num_buffers_allocated = 0;
@@ -887,6 +888,8 @@ ep_buffer_manager_write_event (
887888
bool alloc_new_buffer = false;
888889
EventPipeBuffer *buffer = NULL;
889890
EventPipeThreadSessionState *session_state = NULL;
891+
EventPipeStackContents stack_contents;
892+
EventPipeStackContents *current_stack_contents = NULL;
890893

891894
EP_ASSERT (buffer_manager != NULL);
892895
EP_ASSERT (ep_event != NULL);
@@ -897,12 +900,23 @@ ep_buffer_manager_write_event (
897900
// Before we pick a buffer, make sure the event is enabled.
898901
ep_return_false_if_nok (ep_event_is_enabled (ep_event));
899902

903+
// Check that the payload size is less than 64 KB (max size for ETW events)
904+
if (ep_event_payload_get_size (payload) > 64 * 1024)
905+
{
906+
ep_rt_atomic_inc_int64_t (&buffer_manager->num_oversized_events_dropped);
907+
EventPipeThread *current_thread = ep_thread_get();
908+
ep_rt_spin_lock_handle_t *thread_lock = ep_thread_get_rt_lock_ref (current_thread);
909+
EP_SPIN_LOCK_ENTER (thread_lock, section1)
910+
session_state = ep_thread_get_or_create_session_state (current_thread, session);
911+
ep_thread_session_state_increment_sequence_number (session_state);
912+
EP_SPIN_LOCK_EXIT (thread_lock, section1)
913+
return false;
914+
}
915+
900916
// Check to see an event thread was specified. If not, then use the current thread.
901917
if (event_thread == NULL)
902918
event_thread = thread;
903919

904-
EventPipeStackContents stack_contents;
905-
EventPipeStackContents *current_stack_contents;
906920
current_stack_contents = ep_stack_contents_init (&stack_contents);
907921
if (stack == NULL && ep_event_get_need_stack (ep_event) && !ep_session_get_rundown_enabled (session)) {
908922
ep_walk_managed_stack_for_current_thread (current_stack_contents);
@@ -917,9 +931,9 @@ ep_buffer_manager_write_event (
917931
ep_rt_spin_lock_handle_t *thread_lock;
918932
thread_lock = ep_thread_get_rt_lock_ref (current_thread);
919933

920-
EP_SPIN_LOCK_ENTER (thread_lock, section1)
934+
EP_SPIN_LOCK_ENTER (thread_lock, section2)
921935
session_state = ep_thread_get_or_create_session_state (current_thread, session);
922-
ep_raise_error_if_nok_holding_spin_lock (session_state != NULL, section1);
936+
ep_raise_error_if_nok_holding_spin_lock (session_state != NULL, section2);
923937

924938
buffer = ep_thread_session_state_get_write_buffer (session_state);
925939
if (!buffer) {
@@ -931,7 +945,7 @@ ep_buffer_manager_write_event (
931945
else
932946
alloc_new_buffer = true;
933947
}
934-
EP_SPIN_LOCK_EXIT (thread_lock, section1)
948+
EP_SPIN_LOCK_EXIT (thread_lock, section2)
935949

936950
// alloc_new_buffer is reused below to detect if overflow happened, so cache it here to see if we should
937951
// signal the reader thread
@@ -951,23 +965,23 @@ ep_buffer_manager_write_event (
951965
// This lock looks unnecessary for the sequence number, but didn't want to
952966
// do a broader refactoring to take it out. If it shows up as a perf
953967
// problem then we should.
954-
EP_SPIN_LOCK_ENTER (thread_lock, section2)
968+
EP_SPIN_LOCK_ENTER (thread_lock, section3)
955969
ep_thread_session_state_increment_sequence_number (session_state);
956-
EP_SPIN_LOCK_EXIT (thread_lock, section2)
970+
EP_SPIN_LOCK_EXIT (thread_lock, section3)
957971
} else {
958972
current_thread = ep_thread_get ();
959973
EP_ASSERT (current_thread != NULL);
960974

961975
thread_lock = ep_thread_get_rt_lock_ref (current_thread);
962-
EP_SPIN_LOCK_ENTER (thread_lock, section3)
976+
EP_SPIN_LOCK_ENTER (thread_lock, section4)
963977
ep_thread_session_state_set_write_buffer (session_state, buffer);
964978
// Try to write the event after we allocated a buffer.
965979
// This is the first time if the thread had no buffers before the call to this function.
966980
// This is the second time if this thread did have one or more buffers, but they were full.
967981
alloc_new_buffer = !ep_buffer_write_event (buffer, event_thread, session, ep_event, payload, activity_id, related_activity_id, stack);
968982
EP_ASSERT(!alloc_new_buffer);
969983
ep_thread_session_state_increment_sequence_number (session_state);
970-
EP_SPIN_LOCK_EXIT (thread_lock, section3)
984+
EP_SPIN_LOCK_EXIT (thread_lock, section4)
971985
}
972986
}
973987

‎src/native/eventpipe/ep-buffer-manager.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ struct _EventPipeBufferManager_Internal {
125125
// The total amount of allocations we can do after one sequence
126126
// point before triggering the next one
127127
size_t sequence_point_alloc_budget;
128+
// number of times an event was dropped due to it being too
129+
// large to fit in the 64KB size limit
130+
volatile int64_t num_oversized_events_dropped;
128131

129132
#ifdef EP_CHECKED_BUILD
130133
volatile int64_t num_events_stored;

‎src/tests/tracing/eventpipe/bigevent/bigevent.cs‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Diagnostics.Tracing;
6+
using System.IO;
7+
using System.Linq;
8+
using System.Threading;
9+
using System.Threading.Tasks;
10+
using System.Collections.Generic;
11+
using Microsoft.Diagnostics.Tools.RuntimeClient;
12+
using Microsoft.Diagnostics.Tracing;
13+
using Tracing.Tests.Common;
14+
using Microsoft.Diagnostics.Tracing.Parsers.Clr;
15+
16+
namespace Tracing.Tests.BigEventsValidation
17+
{
18+
19+
public sealed class BigEventSource : EventSource
20+
{
21+
private static string bigString = new String('a', 100 * 1024);
22+
private static string smallString = new String('a', 10);
23+
24+
private BigEventSource()
25+
{
26+
}
27+
28+
public static BigEventSource Log = new BigEventSource();
29+
30+
public void BigEvent()
31+
{
32+
WriteEvent(1, bigString);
33+
}
34+
35+
public void SmallEvent()
36+
{
37+
WriteEvent(2, smallString);
38+
}
39+
}
40+
41+
42+
public class BigEventsValidation
43+
{
44+
public static int Main(string[] args)
45+
{
46+
// This test tries to send a big event (>100KB) and checks that the app does not crash
47+
// See https://github.com/dotnet/runtime/issues/50515 for the regression issue
48+
var providers = new List<Provider>()
49+
{
50+
new Provider("BigEventSource")
51+
};
52+
53+
var configuration = new SessionConfiguration(circularBufferSizeMB: 1024, format: EventPipeSerializationFormat.NetTrace, providers: providers);
54+
return IpcTraceTest.RunAndValidateEventCounts(_expectedEventCounts, _eventGeneratingAction, configuration, _Verify);
55+
}
56+
57+
private static Dictionary<string, ExpectedEventCount> _expectedEventCounts = new Dictionary<string, ExpectedEventCount>()
58+
{
59+
{ "BigEventSource", -1 }
60+
};
61+
62+
private static Action _eventGeneratingAction = () =>
63+
{
64+
// Write 10 big events
65+
for (int i = 0; i < 10; i++)
66+
{
67+
BigEventSource.Log.BigEvent();
68+
}
69+
// Write 10 small events
70+
for (int i = 0; i < 10; i++)
71+
{
72+
BigEventSource.Log.SmallEvent();
73+
}
74+
};
75+
76+
private static Func<EventPipeEventSource, Func<int>> _Verify = (source) =>
77+
{
78+
bool hasSmallEvent = false;
79+
source.Dynamic.All += (TraceEvent data) =>
80+
{
81+
if (data.EventName == "SmallEvent")
82+
{
83+
hasSmallEvent = true;
84+
}
85+
};
86+
return () => hasSmallEvent ? 100 : -1;
87+
};
88+
}
89+
}

‎src/tests/tracing/eventpipe/bigevent/bigevent.csproj‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<TargetFrameworkIdentifier>.NETCoreApp</TargetFrameworkIdentifier>
4+
<OutputType>exe</OutputType>
5+
<CLRTestKind>BuildAndRun</CLRTestKind>
6+
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
7+
<CLRTestPriority>0</CLRTestPriority>
8+
<JitOptimizationSensitive>true</JitOptimizationSensitive>
9+
<UnloadabilityIncompatible>true</UnloadabilityIncompatible>
10+
</PropertyGroup>
11+
<ItemGroup>
12+
<Compile Include="$(MSBuildProjectName).cs" />
13+
<ProjectReference Include="../common/common.csproj" />
14+
</ItemGroup>
15+
</Project>

0 commit comments

Comments
 (0)