Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid reading time stamp counter #1074

Merged
merged 4 commits into from
Sep 7, 2023
Merged

Avoid reading time stamp counter #1074

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7c995f2
Added a static Boolean on the SocketOptions class to avoid using the …
Aug 31, 2023
285b77e
Removing the static flag on SocketOptions for avoidance of reading th…
Sep 6, 2023
6e9e933
Altered the environment variable read to accept any value, rather tha…
rickshaw5724 Sep 6, 2023
743673a
comment fixing
rickshaw5724 Sep 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/NetMQ/Core/Utils/OpCode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ internal static class Opcode

public static bool Open()
{
//Look for an environment variable: "NETQM_SUPPRESS_RDTSC" with a value of "TRUE"
// The application can set this environment variable when this code is running in a system where
// it is not desirable to read the processor's time stamp counter.
// While this is supported in modern CPUs, the technique used for allocating executable memory, copying OP Code
// for the read of the time stamp and invoking the OP Code can be detected as Malware by some anti-virus vendors.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include a link to the original issue here so that we can more easily see the motivation for this feature in future:

Suggested change
// for the read of the time stamp and invoking the OP Code can be detected as Malware by some anti-virus vendors.
// for the read of the time stamp and invoking the OP Code can be detected as Malware by some anti-virus vendors.
// https://github.com/zeromq/netmq/issues/1071

var val = Environment.GetEnvironmentVariable("NETQM_SUPPRESS_RDTSC");
if ("TRUE".Equals(val, StringComparison.OrdinalIgnoreCase))
return false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about supporting any value in this environment variable? It's not uncommon to want to set a variable like this to 1. We could just check for any non-null value.

Suggested change
var val = Environment.GetEnvironmentVariable("NETQM_SUPPRESS_RDTSC");
if ("TRUE".Equals(val, StringComparison.OrdinalIgnoreCase))
return false;
if (Environment.GetEnvironmentVariable("NETQM_SUPPRESS_RDTSC") is not null)
return false;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The downside of this approach would be that setting the value to false would not do what you expect.

I'm happy with whatever you decide.

#if NETSTANDARD1_1_OR_GREATER || NET471_OR_GREATER
if (RuntimeInformation.ProcessArchitecture != Architecture.X86 &&
RuntimeInformation.ProcessArchitecture != Architecture.X64)
Expand Down