[
https://issues.apache.org/jira/browse/QPID-3227?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13026700#comment-13026700
]
[email protected] commented on QPID-3227:
-----------------------------------------------------
bq. On 2011-04-27 15:26:23, Andrew Stitcher wrote:
bq. > I recommend adding in this extra assertion to RdmaIO.cpp:
bq. > This would have caught the original bug.
bq. >
bq. > --- a/qpid/cpp/src/qpid/sys/rdma/RdmaIO.cpp
bq. > +++ b/qpid/cpp/src/qpid/sys/rdma/RdmaIO.cpp
bq. > @@ -213,6 +213,7 @@ namespace Rdma {
bq. > Buffer* ob = buff ? buff : getSendBuffer();
bq. > // Add FrameHeader after frame data
bq. > FrameHeader header(credit);
bq. > + assert(int32_t(ob->dataCount()+FrameHeaderSize) <=
ob->byteCount())
bq. > ::memcpy(ob->bytes()+ob->dataCount(), &header,
FrameHeaderSize);
bq. > ob->dataCount(ob->dataCount()+FrameHeaderSize);
bq. > qp->postSend(ob);
bq. >
bq. >
bq.
bq. Andrew Stitcher wrote:
bq. Ken pointed out that this assertion would only be good for the old
code and the new code needs to add in the reserved space too.
bq.
bq. Kenneth Giusti wrote:
bq. How about something like this (totally untested):
bq.
bq. Index: cpp/src/qpid/sys/rdma/rdma_wrap.h
bq. ===================================================================
bq. --- cpp/src/qpid/sys/rdma/rdma_wrap.h (revision 1097599)
bq. +++ cpp/src/qpid/sys/rdma/rdma_wrap.h (working copy)
bq. @@ -77,6 +77,8 @@
bq. }
bq.
bq. inline void Buffer::dataCount(int32_t s) {
bq. + // catch any attempt to overflow a buffer
bq. + assert(s <= bufferSize + reserved);
bq. sge.length = s;
bq. }
bq.
bq.
bq. This stashes the check in the buffer code itself at the point where
the application sets the length of the output data.
I think we could have both checks in there,
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/667/#review577
-----------------------------------------------------------
On 2011-04-26 20:08:35, Kenneth Giusti wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/667/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-04-26 20:08:35)
bq.
bq.
bq. Review request for qpid, Andrew Stitcher, Gordon Sim, and Chug Rolke.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Prevents buffer overflow bug by explicitly allowing RdmaIO layer to
reserve header space in send buffers.
bq.
bq.
bq. This addresses bug QPID-3227.
bq. https://issues.apache.org/jira/browse/QPID-3227
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. /trunk/qpid/cpp/src/qpid/sys/rdma/RdmaIO.cpp 1096872
bq. /trunk/qpid/cpp/src/qpid/sys/rdma/rdma_wrap.h 1096872
bq. /trunk/qpid/cpp/src/qpid/sys/rdma/rdma_wrap.cpp 1096872
bq.
bq. Diff: https://reviews.apache.org/r/667/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. unit tests & scale testing (by hand using perftest).
bq.
bq.
bq. Thanks,
bq.
bq. Kenneth
bq.
bq.
> rdma layer may allow overrun of send buffers
> --------------------------------------------
>
> Key: QPID-3227
> URL: https://issues.apache.org/jira/browse/QPID-3227
> Project: Qpid
> Issue Type: Bug
> Components: C++ Broker
> Affects Versions: 0.10
> Reporter: Ken Giusti
> Assignee: Ken Giusti
> Fix For: 0.11
>
> Attachments: QPID-3227.patch
>
>
> The rdma driver adds a small trailer to outbound buffers, however the size of
> this header is not accounted for when the buffer's size is passed to the
> codec. If the codec fills all available buffer space, the rdma driver will
> overwrite the end of the buffer when adding the trailer.
> Kudos to Chuck Rolke for helping root-cause this bug!
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:[email protected]
