diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs index 06ecc8d9ed1d72..44fc356c33668a 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs @@ -13,6 +13,11 @@ namespace System.Net { internal sealed class SafeDeleteSslContext : SafeDeleteContext { + // mapped from OSX error codes + private const int OSStatus_writErr = -20; + private const int OSStatus_readErr = -19; + private const int OSStatus_noErr = 0; + private const int OSStatus_errSSLWouldBlock = -9803; private const int InitialBufferSize = 2048; private SafeSslHandle _sslContext; private Interop.AppleCrypto.SSLReadFunc _readCallback; @@ -155,48 +160,66 @@ protected override void Dispose(bool disposing) private unsafe int WriteToConnection(void* connection, byte* data, void** dataLength) { - ulong length = (ulong)*dataLength; - Debug.Assert(length <= int.MaxValue); + // We don't pool these buffers and we can't because there's a race between their us in the native + // read/write callbacks and being disposed when the SafeHandle is disposed. This race is benign currently, + // but if we were to pool the buffers we would have a potential use-after-free issue. + try + { + ulong length = (ulong)*dataLength; + Debug.Assert(length <= int.MaxValue); - int toWrite = (int)length; - var inputBuffer = new ReadOnlySpan(data, toWrite); + int toWrite = (int)length; + var inputBuffer = new ReadOnlySpan(data, toWrite); - _outputBuffer.EnsureAvailableSpace(toWrite); - inputBuffer.CopyTo(_outputBuffer.AvailableSpan); - _outputBuffer.Commit(toWrite); + _outputBuffer.EnsureAvailableSpace(toWrite); + inputBuffer.CopyTo(_outputBuffer.AvailableSpan); + _outputBuffer.Commit(toWrite); + // Since we can enqueue everything, no need to re-assign *dataLength. - // Since we can enqueue everything, no need to re-assign *dataLength. - const int noErr = 0; - return noErr; + return OSStatus_noErr; + } + catch (Exception e) + { + if (NetEventSource.Log.IsEnabled()) + NetEventSource.Error(this, $"WritingToConnection failed: {e.Message}"); + return OSStatus_writErr; + } } private unsafe int ReadFromConnection(void* connection, byte* data, void** dataLength) { - const int noErr = 0; - const int errSSLWouldBlock = -9803; - ulong toRead = (ulong)*dataLength; - - if (toRead == 0) + try { - return noErr; - } + ulong toRead = (ulong)*dataLength; + + if (toRead == 0) + { + return OSStatus_noErr; + } - uint transferred = 0; + uint transferred = 0; - if (_inputBuffer.ActiveLength == 0) - { - *dataLength = (void*)0; - return errSSLWouldBlock; - } + if (_inputBuffer.ActiveLength == 0) + { + *dataLength = (void*)0; + return OSStatus_errSSLWouldBlock; + } - int limit = Math.Min((int)toRead, _inputBuffer.ActiveLength); + int limit = Math.Min((int)toRead, _inputBuffer.ActiveLength); - _inputBuffer.ActiveSpan.Slice(0, limit).CopyTo(new Span(data, limit)); - _inputBuffer.Discard(limit); - transferred = (uint)limit; + _inputBuffer.ActiveSpan.Slice(0, limit).CopyTo(new Span(data, limit)); + _inputBuffer.Discard(limit); + transferred = (uint)limit; - *dataLength = (void*)transferred; - return noErr; + *dataLength = (void*)transferred; + return OSStatus_noErr; + } + catch (Exception e) + { + if (NetEventSource.Log.IsEnabled()) + NetEventSource.Error(this, $"ReadFromConnectionfailed: {e.Message}"); + return OSStatus_readErr; + } } internal void Write(ReadOnlySpan buf)