search

zeromq / netmq

A 100% native C# implementation of ZeroMQ for .NET
Other
2.95k stars 743 forks source link

Added a static Boolean on the SocketOptions class to avoid using the … #1072

Open rickshaw5724 opened 1 year ago

rickshaw5724 commented 1 year ago

…opcode injection technique for reading the time stamp counter.

Fixes #1071

codecov[bot] commented 1 year ago

Codecov Report

Merging #1072 (26b0253) into master (f416a78) will increase coverage by 65.82%. The diff coverage is 0.00%.

:exclamation: Current head 26b0253 differs from pull request most recent head 1a2314a. Consider uploading reports for the commit 1a2314a to get more accurate results

@@             Coverage Diff             @@
##           master    #1072       +/-   ##
===========================================
+ Coverage        0   65.82%   +65.82%     
===========================================
  Files           0      146      +146     
  Lines           0     9074     +9074     
  Branches        0     1450     +1450     
===========================================
+ Hits            0     5973     +5973     
- Misses          0     2500     +2500     
- Partials        0      601      +601
Files Changed Coverage
src/NetMQ/Core/Utils/OpCode.cs 0.00%
src/NetMQ/SocketOptions.cs ø
rickshaw5724 commented 1 year ago

I agree SocketOptions isn't the right place. I'll change it to look for an environment variable

On Tue, Sep 5, 2023, 9:44 PM Drew Noakes @.***> wrote:

@.**** commented on this pull request.

In src/NetMQ/SocketOptions.cs https://github.com/zeromq/netmq/pull/1072#discussion_r1316590551:

  • ///
  • /// If set, the time stamp counter is not read directly through opcode injection,
  • /// rather is used.
  • /// When false, the time stamp counter is read by allocating a few bytes on the heap with
  • /// read/write/execute privilege. OpCode is copied to this allocated memory and invoked to read
  • /// the time stamp counter, (which is a register available on most modern CPUs). While this is
  • /// an accurate way to read the time stamp counter, because it injects code onto the heap, this
  • /// can be detected as a malware technique by some anti-virus defenders.
  • ///
  • public static bool DoNotUseRDTSC;

This doesn't really seem like a socket option, and we don't have any other static options here as far as I can tell.

Also the timing of setting this value is important, as the result of Open is cached in Clock.s_rdtscSupported.

Because of the timing issue, what about using something like an environment variable to control this. For example, if NETQM_SUPPRESS_RDTSC was present, it would disable any attempt to call Opcode.Open() and store a false in s_rdtscSupported. All that could happen in the static constructor of Clock.

— Reply to this email directly, view it on GitHub https://github.com/zeromq/netmq/pull/1072#pullrequestreview-1612191944, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATJKVZLWRNKPIX3DGIOOBZDXY7IP3ANCNFSM6AAAAAA4F76BBU . You are receiving this because you authored the thread.Message ID: @.***>