On Wed, 8 Jul 2020, Philippe Mathieu-Daudé wrote:
On 7/7/20 8:08 PM, Volker Rümelin wrote:
In function oss_read() a read error currently does not exit the
read loop. With no data to read the variable pos will quickly
underflow and a subsequent successful read overwrites memory
outside the buffer. This patch adds the missing break statement
to the error path of the function.
Correct, but ...
To reproduce start qemu with -audiodev oss,id=audio0 and in the
guest start audio recording. After some time this will trigger
an exception.
Fixes: 3ba4066d08 "ossaudio: port to the new audio backend api"
Signed-off-by: Volker Rümelin <vr_q...@t-online.de>
---
audio/ossaudio.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/audio/ossaudio.c b/audio/ossaudio.c
index f88d076ec2..a7dcaa31ad 100644
--- a/audio/ossaudio.c
+++ b/audio/ossaudio.c
@@ -691,6 +691,7 @@ static size_t oss_read(HWVoiceIn *hw, void *buf, size_t len)
len, dst);
break;
}
+ break;
Maybe it would be less confusing if you converted the switch(errno) to an
if then you wouldn't have two senses of break; in close proximity. I was
thinking something like
if (nread == -1) {
if (errno != EINTR && errno != EAGAIN) {
logerr();
}
break; /* from while, which is now clear */
}
}
pos += nread;
... now pos += -1, then the size returned misses the last byte.
Don't you break from loop in if () above this so -1 is never added to pos
after this patch? What happens for EINTR and EAGAIN? Now we break from the
loop for those too, should it be continue; instead?
Regards,
BALATON Zoltan
