Re: [Qemu-devel] [PULL v3 1/3] replay: add record/replay for audio passthrough

["Pavel Dovgalyuk" <dovgaluk@ispras.ru>]

> From: Peter Maydell [mailto:peter.maydell@linaro.org]
> On 1 March 2017 at 15:40, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > From: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> >
> > This patch adds recording and replaying audio data. Is saves synchronization
> > information for audio out and inputs from the microphone.
> >
> > v2: removed unneeded whitespace change
> >
> > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> > Message-id: 20170202055054.4848.94901.stgit@PASHA-ISP.lan02.inno
> >
> > [ kraxel: add qemu/error-report.h include to fix osx build failure ]
> >
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  audio/audio.c            |  9 ++++--
> >  audio/audio.h            |  5 +++
> >  audio/mixeng.c           | 32 ++++++++++++++++++++
> >  docs/replay.txt          |  7 +++++
> >  include/sysemu/replay.h  |  7 +++++
> >  replay/Makefile.objs     |  1 +
> >  replay/replay-audio.c    | 79 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  replay/replay-internal.h |  4 +++
> >  8 files changed, 142 insertions(+), 2 deletions(-)
> >  create mode 100644 replay/replay-audio.c
> >
> > diff --git a/audio/audio.c b/audio/audio.c
> > index c845a44..21f7b0b 100644
> > --- a/audio/audio.c
> > +++ b/audio/audio.c
> > @@ -28,6 +28,7 @@
> >  #include "qemu/timer.h"
> >  #include "sysemu/sysemu.h"
> >  #include "qemu/cutils.h"
> > +#include "sysemu/replay.h"
> >
> >  #define AUDIO_CAP "audio"
> >  #include "audio_int.h"
> > @@ -1387,6 +1388,7 @@ static void audio_run_out (AudioState *s)
> >
> >          prev_rpos = hw->rpos;
> >          played = hw->pcm_ops->run_out (hw, live);
> > +        replay_audio_out(&played);
> >          if (audio_bug (AUDIO_FUNC, hw->rpos >= hw->samples)) {
> >              dolog ("hw->rpos=%d hw->samples=%d played=%d\n",
> >                     hw->rpos, hw->samples, played);
> 
> Hi. Coverity produces a new warning because of this change (CID1390632),
> because it treats the replay file as "tainted data", and complains
> that we trust a value from the file to become a sample count
> passed to audio_capture_mix_and_clear() and eventually used as
> a byte count for a memset.
> 
> Do we trust the replay file to be non-malicious (making this
> a false-positive), or not (in which case we need to sanitize
> or check its contents somehow) ?

Replay file is generated by QEMU and does not affected by the guest system directly.
This file is used by the developer himself (e.g., recording and replaying execution
on the same machine for the analysis or debugging).
Replay file can also be used by testers for bug reporting (e.g., to send bug
reproduction scenario to the developer).

In the case of transferring the file it can be used as an exploit.
But I cannot judge is it a real threat or just inessential one.

Pavel Dovgalyuk