Read only archive ; use https://github.com/JacORB/JacORB/issues for new issues

Bug 958

Summary: BufferManager gives bad buffers
Product: JacORB Reporter: Phil Mesnier <mesnierp>
Component: ORBAssignee: Mailinglist to track bugs <jacorb-bugs>
Status: RESOLVED FIXED    
Severity: major CC: jacorb
Priority: P5    
Version: 3.3   
Hardware: PC   
OS: All   
Attachments: Buffer manager diff
CDR Output Stream diff
An improved variation of the CDR Output Stream patch

Description Phil Mesnier 2013-07-15 17:38:14 UTC
Created attachment 419 [details]
Buffer manager diff

The application was choking when a stream became corrupt, forcing socket closure and loss of message. The stream was corrupted because a previous read request had a buffer too small for desired read. That previous read had pulled the GIOP message header and determined the message length which was between 2^18 and 2^19 bytes. The reader went to the buffer manager, which correctly determined a buffer from slot 18 was needed, and slot 18 had an available buffer. Unfortunately the actual size of that buffer was 0x5A000 rather than the expected 0x7FFFF, and thus insufficient for the necessary read.

The problem is that somehow a buffer was offered to the buffer manager that stored it in the appropriate slot, but without checking that the actual size of the returned buffer actually met the criteria for that slot.

The source of the bad buffer was the CDROutputStream class. In this case, the user had obtained one from the ORB, then used setBuffer() to supply a buffer into which the data was marshaled but then when done, the buffer was returned to the buffer manager.

The solution is 2 fold. First, the buffer manager should validate the size of returned buffers. Second, the CDROutputStream should have a sense of buffer ownership.

Addressing the buffer manager is straight forward. The CDROuputStream's buffer ownership is a little trickier. I think the easiest solution would be to add an overloaded setBuffer method that takes an explicit ownership flag indicating that the stream can return the buffer to the manager or not. The existing 1-arg call would be the same as the 2-arg call with the second argument true:

The only risk is that if the externally set buffer were insufficient for the data being marshaled, the check() method would grow the buffer discarding the original.
Comment 1 Phil Mesnier 2013-07-15 17:40:10 UTC
Created attachment 420 [details]
CDR Output Stream diff
Comment 2 Phil Mesnier 2013-07-30 18:17:28 UTC
Created attachment 421 [details]
An improved variation of the CDR Output Stream patch

Here is a new patch for the CDR Output Stream. The idea here is an application can supply its own buffer, or allow the CDROS object to go to the ORB's buffer manager to obtain one. In either case, ownership of the buffer is passed to the CDROS, which may replace it from the buffer manager, until such time as the application calls release buffer to take back control. The buffer being released DOES NOT have deferred write blocks integrated with it, so for that reason if the application gives a buffer, the deferred write functionality is disabled for that CDROS. If the buffer is selected by size, the application may choose to disable deferred writes.
Comment 3 Nick Cross 2013-09-05 06:03:15 UTC
Patches merged in for 3.3 together with new tests (3505b97 / b15bd90 / af904be )