[PATCH alsa-lib 3/3] pcm: direct: Check xrun/suspend before the slave hwptr update

[Takashi Iwai]

On Thu, 10 Mar 2022 10:35:25 +0100,
S.J. Wang wrote:
> 
> Thanks.
> 
> > 
> > The xrun/suspend may happen at any time and we should check it right
> > before the slave hwptr update.  Otherwise the hwptr value may be screwed
> 
> I think should be "after the slave hwptr update".  If hwptr is screwed,  means
> Suspend happened then check_xrun() can return.
> 
> > and get unexpected large read/write.
> > 
> > Reported-by: S.J. Wang <shengjiu.wang at nxp.com>
> > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> > ---
> >  src/pcm/pcm_dmix.c   | 4 ++--
> >  src/pcm/pcm_dshare.c | 4 ++--
> >  src/pcm/pcm_dsnoop.c | 6 +++---
> >  3 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c index
> > d00d53bef604..111fea157228 100644
> > --- a/src/pcm/pcm_dmix.c
> > +++ b/src/pcm/pcm_dmix.c
> > @@ -426,11 +426,11 @@ static int snd_pcm_dmix_sync_ptr(snd_pcm_t
> > *pcm)
> >         snd_pcm_direct_t *dmix = pcm->private_data;
> >         int err;
> > 
> > +       if (dmix->slowptr)
> > +               snd_pcm_hwsync(dmix->spcm);
> >         err = snd_pcm_direct_check_xrun(dmix, pcm);
> >         if (err < 0)
> >                 return err;
> > -       if (dmix->slowptr)
> > -               snd_pcm_hwsync(dmix->spcm);
> > 
> >         return snd_pcm_dmix_sync_ptr0(pcm, *dmix->spcm->hw.ptr);  } diff --
> 
> Better to get slave_hw_ptr before check_xrun(), like this:
> 
> --- a/src/pcm/pcm_dmix.c
> +++ b/src/pcm/pcm_dmix.c
> @@ -424,15 +424,17 @@ static int snd_pcm_dmix_sync_ptr0(snd_pcm_t *pcm, snd_pcm_uframes_t slave_hw_ptr
>  static int snd_pcm_dmix_sync_ptr(snd_pcm_t *pcm)
>  {
>         snd_pcm_direct_t *dmix = pcm->private_data;
> +       snd_pcm_uframes_t slave_hw_ptr;
>         int err;
> 
>         if (dmix->slowptr)
>                 snd_pcm_hwsync(dmix->spcm);
> +       slave_hw_ptr = *dmix->spcm->hw.ptr;
>         err = snd_pcm_direct_check_xrun(dmix, pcm);
>         if (err < 0)
>                 return err;
> 
> -       return snd_pcm_dmix_sync_ptr0(pcm, *dmix->spcm->hw.ptr);
> +       return snd_pcm_dmix_sync_ptr0(pcm, slave_hw_ptr);
>  }

Makes sense.  I'll respin v2 with those changes.


thanks,

Takashi