bugGNUstep - Bugs: bug #40585, Releasing a GSTLSHandle without...

Group
 
 

bug #40585: Releasing a GSTLSHandle without first sending -sslDisconnect will segfault

Submitter:  Saso Kiselkov <skiselkov>
Submitted:  Thu 14 Nov 2013 11:35:06 PM UTC
   
 
Category:  Base/Foundation Severity:  3 - Normal
Item Group:  Bug Status:  Fixed
Privacy:  Public Assigned to:  None
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Tue 19 Nov 2013 10:38:59 PM UTC, comment #1: 

Thanks … I applied that patch.

Richard Frith-Macdonald <CaS>
Group Member
Thu 14 Nov 2013 11:35:06 PM UTC, original submission:  

Releasing a GSTLSHandle (created from +[NSFileHandle sslClass]) after the TLS handshake will result in a segfault. The backtrace chain looks as follows:

 fffffd7ffc4323ab GSTLSHandlePull () + 7b
 fffffd7ffbdb2692 _gnutls_stream_read () + 1d9
 fffffd7ffbdb2a53 _gnutls_read () + 64
 fffffd7ffbdb2da8 _gnutls_io_read_buffered () + 1bf
 fffffd7ffbdab8cd recv_headers () + 98
 fffffd7ffbdac09c _gnutls_recv_in_buffers () + 266
 fffffd7ffbdacca8 _gnutls_recv_int () + 204
 fffffd7ffbda99a0 gnutls_bye () + 168
 fffffd7ffc3ab81b _i_GSTLSSession__disconnect () + 4d
 fffffd7ffc3ab881 _i_GSTLSSession__finalize () + 30
 fffffd7ffc3ab6e9 _i_GSTLSSession__dealloc () + 30
 fffffd7ffc485680 _i_NSObject__release () + 40
 fffffd7ffc4325b9 _i_GSTLSHandle__dealloc () + 7f
 fffffd7ffc485680 _i_NSObject__release () + 40
                              ^------ invoked by last -release of the object

The cause is twofold:

1) On a read() error, GSTLSHandlePull dereferences tls->session->session. However, since -[GSTLSHandle dealloc] uses DESTROY for its ivars (including session), these references are already `nil' before the call to -[GSTLSSession release], and hence a NULL-pointer dereference ensues.

2) The primary trigger for this is the read() failure. This is an EAGAIN error because GSTLSHandle sets its socket to O_NONBLOCK, and so the TLS teardown process needs to poll() before read()ing.

The attached patch resolves both aspects. First, inside of -[GSTLSHandle dealloc] it switches the socket back into blocking mode, anticipating blocking in the possible upcoming the TLS teardown. Next, in case an I/O error does really occur, it first TEST_RELEASE()s the ivars (which invokes their -release methods before `nil'ing out the references) and then `nil's them.

Saso Kiselkov <skiselkov>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #29621:  gnustep-tls-teardown.patch added by skiselkov (588B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by CaS (Posted a comment)
  • -email is unavailable- added by skiselkov (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-11-19 CaS StatusNone Fixed
        Open/ClosedOpen Closed
    2013-11-14 skiselkov Attached File- Added gnustep-tls-teardown.patch, #29621

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code