Re: double free or corruption (out) in iscsi virtual machine

2024-02-15 Thread m_o_bz
yep,I found this commit too,  already patch this commit and test it, the bug 
can not reproduce any more




 原始邮件 
发件人:Fiona Ebner 
时间:2024年2月15日 18:29
收件人:M_O_Bz ,qemu-block 
抄送:"deepa.srinivasan" ,qemu-devel 
,ronniesahlberg ,pbonzini 
,pl 
主题:Re: double free or corruption (out) in iscsi virtual machine

>Am 17.01.24 um 08:23 schrieb M_O_Bz:
>> Basic Info:
>> 1. Issue: I got a " double free or corruption (out)", head for
>> attachment debug.log for details, the debug.log print the backtrace of
>> one virtual machine
>> 2. Reproduce: currently I cann't destribe how to reproduce this bug,
>> because it's in my productive enviroment which include some special stuffs
>> 3. qemu version:  I'm using is qemu-6.0.1
>> 4. qemu ccmdline in short:(checkout detail in the virtual machine log
>> message)
>
>Hi,
>sounds like it might be the issue fixed by:
>https://github.com/qemu/qemu/commit/5080152e2ef6cde7aa692e29880c62bd54acb750
>
>Best Regards,
>Fiona


[PATCH] HDA codec: Fix wanted_r/w position overflow

2023-08-17 Thread M_O_Bz
From: zeroway 

when the duration now - buft_start reach to some kind of value,
which will get the multiply hda_bytes_per_second(st) * (now - buft_start) 
overflow,
instead of calculate the wanted_r/wpos from start time to current time,
here calculate the each timer tick delta data first in wanted_r/wpos_delta,
and sum it all to wanted_r/wpos to avoid the overflow

Signed-off-by: zeroway 
---
 hw/audio/hda-codec.c | 24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index c51d8ba617..747188221a 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -169,6 +169,8 @@ struct HDAAudioStream {
 uint8_t buf[8192]; /* size must be power of two */
 int64_t rpos;
 int64_t wpos;
+int64_t wanted_rpos;
+int64_t wanted_wpos;
 QEMUTimer *buft;
 int64_t buft_start;
 };
@@ -226,16 +228,18 @@ static void hda_audio_input_timer(void *opaque)
 int64_t wpos = st->wpos;
 int64_t rpos = st->rpos;
 
-int64_t wanted_rpos = hda_bytes_per_second(st) * (now - buft_start)
+int64_t wanted_rpos_delta = hda_bytes_per_second(st) * (now - buft_start)
   / NANOSECONDS_PER_SECOND;
-wanted_rpos &= -4; /* IMPORTANT! clip to frames */
+st->wanted_rpos += wanted_rpos_delta;
+st->wanted_rpos &= -4; /* IMPORTANT! clip to frames */
 
-if (wanted_rpos <= rpos) {
+st->buft_start = now;
+if (st->wanted_rpos <= rpos) {
 /* we already transmitted the data */
 goto out_timer;
 }
 
-int64_t to_transfer = MIN(wpos - rpos, wanted_rpos - rpos);
+int64_t to_transfer = MIN(wpos - rpos, st->wanted_rpos - rpos);
 while (to_transfer) {
 uint32_t start = (rpos & B_MASK);
 uint32_t chunk = MIN(B_SIZE - start, to_transfer);
@@ -290,16 +294,18 @@ static void hda_audio_output_timer(void *opaque)
 int64_t wpos = st->wpos;
 int64_t rpos = st->rpos;
 
-int64_t wanted_wpos = hda_bytes_per_second(st) * (now - buft_start)
+int64_t wanted_wpos_delta = hda_bytes_per_second(st) * (now - buft_start)
   / NANOSECONDS_PER_SECOND;
-wanted_wpos &= -4; /* IMPORTANT! clip to frames */
+st->wanted_wpos += wanted_wpos_delta;
+st->wanted_wpos &= -4; /* IMPORTANT! clip to frames */
 
-if (wanted_wpos <= wpos) {
+st->buft_start = now;
+if (st->wanted_wpos <= wpos) {
 /* we already received the data */
 goto out_timer;
 }
 
-int64_t to_transfer = MIN(B_SIZE - (wpos - rpos), wanted_wpos - wpos);
+int64_t to_transfer = MIN(B_SIZE - (wpos - rpos), st->wanted_wpos - wpos);
 while (to_transfer) {
 uint32_t start = (wpos & B_MASK);
 uint32_t chunk = MIN(B_SIZE - start, to_transfer);
@@ -420,6 +426,8 @@ static void hda_audio_set_running(HDAAudioStream *st, bool 
running)
 int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 st->rpos = 0;
 st->wpos = 0;
+st->wanted_rpos = 0;
+st->wanted_wpos = 0;
 st->buft_start = now;
 timer_mod_anticipate_ns(st->buft, now + HDA_TIMER_TICKS);
 } else {
-- 
2.35.1




回复:Re: hda codec issue

2023-08-17 Thread M_O_Bz


I try to refine the code like below(only show the code i modify meaningful),  I 
calculate the each 1ms timer wanted_wpos which I named it wanted_wpos_delta 
first instead of caculate the hole duration wanted_wpos in one time
and summer all of them to calcute the wanted_wpos


   /* calcute each 1ms delta wanted pos */

int64_t wanted_wpos_delta = hda_bytes_per_second(st) * (now - 
st->last_start) 

/ NANOSECONDS_PER_SECOND;



/* summer it all to wanted_wpos */
st->wanted_wpos += wanted_wpos_delta;


error_report("delta = %ld, wanted = %ld wpos = %ld, 
now=%ld,buft=%ld,last=%ld\n",
wanted_wpos_delta, st->wanted_wpos, wpos, now, buft_start, 
st->last_start);


/* mark the last timer timestamp */
st->last_start = now; 



in the function hda_timer_sync_adjust, i add the corr to the last_start value

st->last_start += corr;



the output  log like below:
2023-08-17T05:29:17.730066Z qemu-fix: delta = 196, wanted = 22027480 wpos = 
22027284, now=213350279414,buft=97328733406,last=213349253800
2023-08-17T05:29:17.731090Z qemu-fix: delta = 196, wanted = 22027676 wpos = 
22027480, now=213351303295,buft=97328733406,last=213350279414
2023-08-17T05:29:17.732179Z qemu-fix: delta = 209, wanted = 22027884 wpos = 
22027676, now=213352392204,buft=97328733406,last=213351303295
2023-08-17T05:29:17.733201Z qemu-fix: delta = 196, wanted = 22028080 wpos = 
22027884, now=213353414012,buft=97328733406,last=213352392204
2023-08-17T05:29:17.734234Z qemu-fix: delta = 198, wanted = 22028276 wpos = 
22028080, now=213354447484,buft=97328733406,last=213353414012


I currently test with playback, it works and seems to work as expected.


What about this solution for this bug? any suggestion for me?












在 2023-08-16 15:04:14,"Volker Rümelin"  写道:
>Cc: qemu-devel@nongnu.org
>
>> cc Volker Rümelin
>>
>> ==
>>
>> so I was curious about:
>> 1. why using wpos, rpos which will increasing along the time, which 
>> may overflow in the feature time?
>
>wpos and rpos are 64 bit integers. At a sample rate of 96kHz and with 32 
>bit stereo audio frames they overflow after 2^63 bytes / (8 bytes/frame 
>* 96000 frames/s) = 1.2E13 s = 380561.23 years. I don't think you will 
>notice an overflow.
>
>But you're right anyway. The wanted_rpos and wanted_wpos calculation 
>overflows quite early. At a sample rate of 44100kHz and with 16 bit 
>stereo samples the multiplication hda_bytes_per_second(st) * (now - 
>buft_start) overflows after 2^63 bytes / 176400 bytes/ns = 5,23E13 ns = 
>14.58 hours.
>
>I don't think your fix below is correct. There are certainly ways to 
>prevent the overflow without resetting the buffer.
>
>> 2. how to fix the wanted_wpos or wanted_rpos, I current don't 
>> understand the  wanted_pos mean and how it work to optimize the codec
>
>This is a jitter-buffer implementation to decouple the audio backend 
>updates from the hda device updates. The code splits large  >10ms sized 
>audio packets into 1ms sized audio packets.
>
>With best regards,
>Volker
>
>> ===
>>
>> Currently  I got Playback and Capture issue
>>
>> I test Playing and recoding for a very long time, Playback issue is 
>> hard to reproduce, Capture issue is easy to reproduce for recording 
>> about 14 hours
>>
>> Playback issue current can only be found on qemu 2.11, which I found 
>> the code below (remove some unnecessary)
>>
>> wanted_wpos overflow, and is always little then the wpos,(current wpos 
>> - rpos = 0) so qemu will no long get data from vm, as a result
>>
>> static void hda_audio_output_timer(void *opaque)
>> {
>> .
>>
>> int64_t wanted_wpos = hda_bytes_per_second(st) * (now - 
>> buft_start) > overflow here
>>   / NANOSECONDS_PER_SECOND;
>> wanted_wpos &= -4; /* IMPORTANT! clip to frames */
>>
>> if (wanted_wpos <= wpos) {
>> /* we already received the data */
>> goto out_timer;
>> }
>> }
>>
>>
>> Capture issue: Just record all the time
>> (i using qemu 6.0 to test) I add code below to the input callback 
>> function like below, the issue dispear
>> hda_audio_input_cb
>> if (wpos - rpos == B_SIZE) {
>> /* drop buffer, reset timer adjust */
>> st->rpos = 0;
>> st->wpos = 0;
>> st->buft_start = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>> error_report("input drop buffer, reset timer adjust\n");
>> return;
>> }
>>
>>
>>
>>
>>
>>