fixed for AddRingMessage in alsa,nas,and oss

[Jeremy Shaw]

Hello,

alsa, nas, oss, and arts all have a major bug in the
*_AddRingMessage() function. Namely, it is very likely that some
unread ring buffer messages will be overwritten if the ring buffer is
resized.

If you look at the current code that resizes the ring buffer:

    if ((omr->msg_toget == ((omr->msg_tosave + 1) % omr->ring_buffer_size)))
    {
	omr->ring_buffer_size += OSS_RING_BUFFER_INCREMENT;
	TRACE("omr->ring_buffer_size=%d\n",omr->ring_buffer_size);
	omr->messages = HeapReAlloc(GetProcessHeap(),0,omr->messages, omr->ring_buffer_size * sizeof(OSS_MSG));
    }


you will see that it detects that the msg_tosave index is about to
catch up to the msg_toget, and adds more space to the end of the array
that holds the ring buffer messages.

So, for example, if tosave "points" to entry 3, and toget "points" to
4, then the buffer clearly needs more space, otherwise, the tosave
index will start pointing to messages that have not been read yet.

The existing code makes the ring buffer bigger, but doesn't move the
data around or update the indexes. So in the previous example, after
the ring buffer size is increased tosave will still "point" to entry 3
and toget will still "point" to index 4. So, if more messages are
added to the ring buffer, the unread messages (starting at index 4)
will be overwritten.

This patch fixes the bug by moving the data around in the array after
the buffer size is increased so that the empty messages are inbetween
the tosave and toget indexes.

winearts also needs this patch, but I will be submitting a winearts
patch for wave-in support later this week that includes the fix. alsa
0.5 does not include the support for buffer resizing, and libjack does
not use the ring buffer code at all.

Jeremy Shaw.


--- orig/dlls/winmm/winealsa/audio.c
+++ mod/dlls/winmm/winealsa/audio.c
@@ -563,9 +563,22 @@
     EnterCriticalSection(&omr->msg_crst);
     if ((omr->msg_toget == ((omr->msg_tosave + 1) % omr->ring_buffer_size)))
     {
+	int old_ring_buffer_size = omr->ring_buffer_size;
 	omr->ring_buffer_size += ALSA_RING_BUFFER_INCREMENT;
 	TRACE("omr->ring_buffer_size=%d\n",omr->ring_buffer_size);
 	omr->messages = HeapReAlloc(GetProcessHeap(),0,omr->messages, omr->ring_buffer_size * sizeof(ALSA_MSG));
+	/* Now we need to rearrange the ring buffer so that the new
+	   buffers just allocated are in between omr->msg_tosave and
+	   omr->msg_toget.
+	*/
+	if (omr->msg_tosave < omr->msg_toget)
+	{
+	    memmove(&(omr->messages[omr->msg_toget + ALSA_RING_BUFFER_INCREMENT]),
+		    &(omr->messages[omr->msg_toget]),
+		    sizeof(ALSA_MSG)*(old_ring_buffer_size - omr->msg_toget)
+		    );
+	    omr->msg_toget += ALSA_RING_BUFFER_INCREMENT;
+	}
     }
     if (wait)
     {


--- orig/dlls/winmm/winenas/audio.c
+++ mod/dlls/winmm/winenas/audio.c
@@ -454,9 +454,22 @@
     EnterCriticalSection(&mr->msg_crst);
     if ((mr->msg_toget == ((mr->msg_tosave + 1) % mr->ring_buffer_size)))
     {
+	int old_ring_buffer_size = mr->ring_buffer_size;
 	mr->ring_buffer_size += NAS_RING_BUFFER_INCREMENT;
 	TRACE("omr->ring_buffer_size=%d\n",mr->ring_buffer_size);
 	mr->messages = HeapReAlloc(GetProcessHeap(),0,mr->messages, mr->ring_buffer_size * sizeof(RING_MSG));
+	/* Now we need to rearrange the ring buffer so that the new
+	   buffers just allocated are in between mr->msg_tosave and
+	   mr->msg_toget.
+	*/
+	if (mr->msg_tosave < mr->msg_toget)
+	{
+	    memmove(&(mr->messages[mr->msg_toget + NAS_RING_BUFFER_INCREMENT]),
+		    &(mr->messages[mr->msg_toget]),
+		    sizeof(RING_MSG)*(old_ring_buffer_size - mr->msg_toget)
+		    );
+	    mr->msg_toget += NAS_RING_BUFFER_INCREMENT;
+	}
     }
     if (wait)
     {


--- orig/dlls/winmm/wineoss/audio.c
+++ mod/dlls/winmm/wineoss/audio.c
@@ -909,9 +909,22 @@
     EnterCriticalSection(&omr->msg_crst);
     if ((omr->msg_toget == ((omr->msg_tosave + 1) % omr->ring_buffer_size)))
     {
+	int old_ring_buffer_size = omr->ring_buffer_size;
 	omr->ring_buffer_size += OSS_RING_BUFFER_INCREMENT;
 	TRACE("omr->ring_buffer_size=%d\n",omr->ring_buffer_size);
 	omr->messages = HeapReAlloc(GetProcessHeap(),0,omr->messages, omr->ring_buffer_size * sizeof(OSS_MSG));
+	/* Now we need to rearrange the ring buffer so that the new
+	   buffers just allocated are in between omr->msg_tosave and
+	   omr->msg_toget.
+	*/
+	if (omr->msg_tosave < omr->msg_toget)
+	{
+	    memmove(&(omr->messages[omr->msg_toget + OSS_RING_BUFFER_INCREMENT]),
+		    &(omr->messages[omr->msg_toget]),
+		    sizeof(OSS_MSG)*(old_ring_buffer_size - omr->msg_toget)
+		    );
+	    omr->msg_toget += OSS_RING_BUFFER_INCREMENT;
+	}
     }
     if (wait)
     {