From 41a926a9290e2a608cf5c77b27b1790841938894 Mon Sep 17 00:00:00 2001 From: Gerald Pohn Date: Fri, 11 Sep 2026 11:31:33 +0200 Subject: [PATCH 1/2] Fix thread-unsafe LinkedList access in OpenSSLConnector pending buffer lists (GML-1187) --- .../OpenSSL/OpenSSLConnector.cs | 60 +++++++++++++++---- 1 file changed, 49 insertions(+), 11 deletions(-) diff --git a/src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs b/src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs index da21d9a..5f513a4 100644 --- a/src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs +++ b/src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs @@ -33,6 +33,8 @@ public class OpenSSLConnector : IDisposable private readonly byte[] m_buffer = new byte[4096]; private readonly DataBufferList m_pendingWriteList; private readonly DataBufferList m_pendingReadList; + private readonly object m_readListLock = new object(); + private readonly object m_writeListLock = new object(); private bool m_fatalError; private bool m_disposed; @@ -277,8 +279,16 @@ protected void RunSSL() while (!m_fatalError && ((!m_readRequired && dataToWrite) || dataToRead)) { - int pendingReadBefore = m_pendingReadList.Count; - int pendingWriteBefore = m_pendingWriteList.Count; + int pendingReadBefore; + int pendingWriteBefore; + lock (m_readListLock) + { + pendingReadBefore = m_pendingReadList.Count; + } + lock (m_writeListLock) + { + pendingWriteBefore = m_pendingWriteList.Count; + } bool readRequiredBefore = m_readRequired; var bioPendingBefore = Native.BIO_ctrl_pending(m_pBioOut); @@ -304,8 +314,18 @@ protected void RunSSL() GetPendingOperations(ref dataToRead, ref dataToWrite); var bioPendingAfter = Native.BIO_ctrl_pending(m_pBioOut); - if (pendingReadBefore == m_pendingReadList.Count && - pendingWriteBefore == m_pendingWriteList.Count && + int pendingReadAfter; + int pendingWriteAfter; + lock (m_readListLock) + { + pendingReadAfter = m_pendingReadList.Count; + } + lock (m_writeListLock) + { + pendingWriteAfter = m_pendingWriteList.Count; + } + if (pendingReadBefore == pendingReadAfter && + pendingWriteBefore == pendingWriteAfter && readRequiredBefore == m_readRequired && bioPendingBefore == bioPendingAfter) { @@ -341,8 +361,14 @@ public void ReadCompleted(byte[] pData, int dataLen) private void GetPendingOperations(ref bool dataToRead, ref bool dataToWrite) { - dataToRead = m_pendingReadList.Count > 0; - dataToWrite = m_pendingWriteList.Count > 0; + lock (m_readListLock) + { + dataToRead = m_pendingReadList.Count > 0; + } + lock (m_writeListLock) + { + dataToWrite = m_pendingWriteList.Count > 0; + } } private void PerformRead() @@ -437,23 +463,35 @@ private void UseData(DataBufferList list, DataBuffer pBuffer, int result) private void AppendBuffer(DataBufferList list, DataBuffer pBuffer) { - list.AddLast(pBuffer); + object lockObject = (list == m_pendingReadList) ? m_readListLock : m_writeListLock; + lock (lockObject) + { + list.AddLast(pBuffer); + } } DataBuffer GetNextBuffer(DataBufferList list) { DataBuffer head = null; - if (list.Count > 0) + object lockObject = (list == m_pendingReadList) ? m_readListLock : m_writeListLock; + lock (lockObject) { - head = list.First.Value; - list.RemoveFirst(); + if (list.Count > 0) + { + head = list.First.Value; + list.RemoveFirst(); + } } return head; } void PutbackBuffer(DataBufferList list, DataBuffer pBuffer) { - list.AddFirst(pBuffer); + object lockObject = (list == m_pendingReadList) ? m_readListLock : m_writeListLock; + lock (lockObject) + { + list.AddFirst(pBuffer); + } } /// From 626fe2120ba5ab5452cb7414511b57a1b76c0180 Mon Sep 17 00:00:00 2001 From: PG80 <98319494+Stiwa-PohG@users.noreply.github.com> Date: Wed, 7 Oct 2026 10:24:35 +0200 Subject: [PATCH 2/2] Update OpenSSLConnector.cs Because Monitor is re-entrant, same-thread nesting of RunSSL (a real possibility given the callback-driven data flow) is fine; the gate just prevents a second thread from entering the whole processing path. Keep AppendBuffer's internal list lock only for any producer that appends without driving RunSSL. --- .../OpenSSL/OpenSSLConnector.cs | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs b/src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs index 5f513a4..41ff016 100644 --- a/src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs +++ b/src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs @@ -343,20 +343,24 @@ protected void RunSSL() } } - public void Write(byte[] pData,int dataLen) + private readonly object m_sslGate = new object(); + + public void Write(byte[] pData, int dataLen) { - DataBuffer pBuffer = new DataBuffer(pData, dataLen); - AppendBuffer(m_pendingWriteList, pBuffer); - - RunSSL(); + lock (m_sslGate) + { + AppendBuffer(m_pendingWriteList, new DataBuffer(pData, dataLen)); + RunSSL(); + } } - + public void ReadCompleted(byte[] pData, int dataLen) { - DataBuffer pBuffer = new DataBuffer(pData, dataLen); - AppendBuffer(m_pendingReadList, pBuffer); - - RunSSL(); + lock (m_sslGate) + { + AppendBuffer(m_pendingReadList, new DataBuffer(pData, dataLen)); + RunSSL(); + } } private void GetPendingOperations(ref bool dataToRead, ref bool dataToWrite)