>> diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
>> index a144a3f68e9e..e839459916ca 100644
>> --- a/sound/core/pcm_lib.c
>> +++ b/sound/core/pcm_lib.c
>> @@ -2127,11 +2127,30 @@ int pcm_lib_apply_appl_ptr(struct
>> snd_pcm_substream *substream,
>> {
>> struct snd_pcm_runtime *runtime = substream->runtime;
>> snd_pcm_uframes_t old_appl_ptr = runtime->control->appl_ptr;
>> + snd_pcm_sframes_t diff;
>> int ret;
>>
>> if (old_appl_ptr == appl_ptr)
>> return 0;
>>
>> + /*
>> + * check if a rewind is requested by the application, after
>> + * verifying the new appl_ptr is in the 0..boundary range
>> + */
>> + if (substream->runtime->info & SNDRV_PCM_INFO_NO_REWINDS) {
>> + if (appl_ptr >= runtime->boundary)
>> + appl_ptr -= runtime->boundary;
>
> The boundary check can (or should) be done unconditionally.
> It was too naive to assume a sane appl_ptr passed always.
> And, it can rather return an error. So,
>
> if (appl_ptr >= runtime->boundary)
> return -EINVAL;
ok, but that would be a separate patch then since it impacts all users,
even without the NO_REWINDS.
>
> /* check if a rewind is requested by the application */
> if (substream->runtime->info & SNDRV_PCM_INFO_NO_REWINDS) {
> diff = appl_ptr - old_appl_ptr;
> ....
>
>> + if (diff >= 0) {
>> + if (diff > runtime->buffer_size)
>> + return 0;
>> + } else {
>> + if (runtime->boundary + diff > runtime->buffer_size)
>> + return 0;
>
> I'm not sure whether we should return 0 here. In snd_pcm_rewind() it
> returns 0 due to application breakage, though.
We could return -EINVAL indeed, that would keep the work-around in place
for PulseAudio.
Even for other uses, it's not so bad: the selection of NO_REWINDS is an
opt-in, and if a rewind still occurs a big fail would help detect a
configuration issue.
