On Tue, 09 Oct 2018 16:35:52 +0200,
Wenwen Wang wrote:
>
> In snd_intel8x0_codec_semaphore(), the parameter 'codec' is firstly
> checked to see whether it is greater than 2. If yes, an error code EIO is
> returned. Otherwise, 'chip->in_sdin_init' is further checked. If
> 'chip->in_sdin_init' is not zero, 'codec' is updated immediately with
> 'chip->codec_isr_bits'. That means, the parameter 'codec' is not used in
> this branch. Actually, it is only used in the else branch when
> 'chip->in_sdin_init' is zero. Thus, the check on the parameter 'codec' is
> redundant if 'chip->in_sdin_init' is not zero. This can cause potential
> incorrect execution in this function. Suppose the parameter 'codec' is 3,
> which is greater than 2, and 'chip->in_sdin_init' is not zero. The current
> version of this function will return EIO after the first check because
> 'codec' is greater than 2. However, since 'codec' will be updated in the
> following execution when 'chip->in_sdin_init' is not zero, this check will
> be meaningless and the execution should continue, instead of returning the
> error code EIO.
>
> This patch avoids the above issue by moving the check on the parameter
> 'codec' to the else branch of the if statement that checks
> 'chip->in_sdin_init'.
>
> Signed-off-by: Wenwen Wang <wang6495@xxxxxxx>
Passing codec > 2 is just incorrect, no matter whether it's in
in_sdin_init state of not. In the in_sdin_init state, it's supposed
to be ignored, but still passing such a value is already wrong.
That said, there is no need to skip the check.
Of course, if your patch fixes any real issue (i.e. a bug), I'll
happily apply the patch. In that case, please show the exact use case
that hits the bug.
thanks,
Takashi
> ---
> sound/pci/intel8x0.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c
> index 5ee468d..f619e58 100644
> --- a/sound/pci/intel8x0.c
> +++ b/sound/pci/intel8x0.c
> @@ -515,13 +515,13 @@ static int snd_intel8x0_codec_semaphore(struct intel8x0 *chip, unsigned int code
> {
> int time;
>
> - if (codec > 2)
> - return -EIO;
> if (chip->in_sdin_init) {
> /* we don't know the ready bit assignment at the moment */
> /* so we check any */
> codec = chip->codec_isr_bits;
> } else {
> + if (codec > 2)
> + return -EIO;
> codec = chip->codec_bit[chip->ac97_sdin[codec]];
> }
>
> --
> 2.7.4
>
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
