Uploaded image for project: 'Core ReactOS'
  1. Core ReactOS
  2. CORE-17526

AFD handles buffer arrays incorrectly for Datagram send operations(and recv)

    XMLWordPrintable

Details

    • Bug
    • Resolution: Unresolved
    • Major
    • None
    • Networking

    Description

      SYNOPSIS - AFD seems to lack PAD code to handle packets built using its array, currently only sends first array member (apparently rarely used ?) - All Vectored IO operations on sockets malfunction or file on ReactOS. Recv returns WSAEMSGSIZE if buffercount > 1.

      created PR with patches to assemble(gather) buffers in send operations, recv fix pending.

      I have been trying to get SAMBA4 running on ROS but I believe I have run into a bug in AFD. Based on reviewing the code and adding instrumentation to trace it I believe the issue stems from mishandling the buffer structures sent in as parameters via winsock. WSASendTo accepts a 'buffer array' for its data source, which arrives as a pointer to the base of the array and a count of buffer items. The array itself is a simple list of structures containing sizes and pointers to data blocks (plural). Winsock validates the parameters and passes them down to AFD which is responsible for locking and duplicating the buffers for the driver layer. In order to work properly and minimize the use of pool memory, AFD would need to perform a 'gather' operation on the buffers bringing them into a single new MDL allocation making the data contiguous and stripping away slack space (the source buffers need not be contiguous, that is not a documented requirement for the source buffers).

      Unfortunately the code in AFD does not seem handle the process of cloning the 'buffer array' properly, and the Datagram function in AFD's  write.c call TdiSendDatagram() using the size and address of the first buffer in the array and simply ignore the remaining elements. As a result of this combination the AFD code works fine for buffer arrays with a single element, but fails badly with any buffer arrays that actually do contain multiple elements sending only the first element which the application detects as an incomplete packet and discards. the code does not attempt to send the remaining buffers separately, it simply discards them.

      Samba4 uses this method for assembling its control messages building them from several pieces stored in different places the first of which is a short ID cookie. The receiver gets that but discards the packet as invalid because there is no payload. As a result even the unit test application for the messaging library in Samba4 fails to run on ROS even though it works correctly on Windows and does not make use any newer API functions not yet implemented in ROS.

      I'm pretty sure this condition results from two separate bugs, but the biggest issue is the mishandling of buffers in the AFD LockBuffers() function. When combined with the erroneous use of a single buffer in  AfdConnectedSocketWriteData() and AfdPacketSocketWriteData() which never check the buffer count parameter passed as an input parameter yields broken datagram send functions.

      I created a small hack to properly determine the array length before calling TdiSendDatagram(), but that did not prove to work because the buffers are allocated individually and teh slack space is not removed so copying the proper number of data bytes is not enough.

      This second failure is what gives me the impression that LockBuffers needs significant revision as well. Either the allocations are not consecutive in memory, or the 'slack' space between them is not being eliminated resulting in sending the slack space rather than the data. I am doing some testing to see if sending the entire allocation area resolves the issue temporarily, but that is far from efficient since the buffers appear to be pre-allocated large enough to house an entire packet payload, and their combined length makes for a big wasteful datagram, and may even exceed the allowable payload size for a datagram packet.

      Attachments

        1. WINDOWS10_UDP_BUFFARRAY_TEST.png
          WINDOWS10_UDP_BUFFARRAY_TEST.png
          22 kB
        2. UDP_Server.exe
          32 kB
        3. UDP_OK.png
          UDP_OK.png
          94 kB
        4. UDP_Fails.png
          UDP_Fails.png
          113 kB
        5. UDP_DGRAM.zip
          3 kB
        6. UDP_Client.exe
          32 kB
        7. ReactOS_4.13_UDP_BUFFARRAY_TEST.png
          ReactOS_4.13_UDP_BUFFARRAY_TEST.png
          21 kB
        8. CORE-17526-fail.png
          CORE-17526-fail.png
          66 kB
        9. 4.15-afd.zip
          186 kB

        Issue Links

          Activity

            People

              Unassigned Unassigned
              bugdude bugdude
              Votes:
              3 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated: