linux-kernel - Re: [PATCH] sound:hdmi:fix without unlocked before return

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <475cb7a9-0992-3a79-a0ad-d3f8922ba3cb@web.de>
Date:   Sun, 26 Apr 2020 19:00:54 +0200
From:   Markus Elfring <Markus.Elfring@....de>
To:     Wu Bo <wubo40@...wei.com>, alsa-devel@...a-project.org
Cc:     linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
        Aaron Plattner <aplattner@...dia.com>,
        Feilong Lin <linfeilong@...wei.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Kai Vehmanen <kai.vehmanen@...ux.intel.com>,
        Libin Yang <libin.yang@...ux.intel.com>,
        Nikhil Mahale <nmahale@...dia.com>,
        Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        Takashi Iwai <tiwai@...e.com>,
        Zhiqiang Liu <liuzhiqiang26@...wei.com>
Subject: Re: [PATCH] sound:hdmi:fix without unlocked before return

> After add sanity check to pass klockwork check,
> The spdif_mutex should be unlock before return true
> in check_non_pcm_per_cvt().

How do you think about a wording variant like the following?

   Subject:
   [PATCH v2] ALSA: hda/hdmi: Unlock a mutex always before returning from check_non_pcm_per_cvt()

   Change description (according to a solution alternative):
   The exception handling was incomplete in an if branch for
   a null pointer check of the variable “spdif”.
   Thus assign an appropriate value to the local variable “non_pcm”
   by using a conditional operator instead.
   A mutex is also appropriately unlocked then.


…
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -1848,8 +1848,10 @@ static bool check_non_pcm_per_cvt(struct hda_codec *codec, hda_nid_t cvt_nid)
…
> -	if (WARN_ON(spdif == NULL))
> +	if (WARN_ON(spdif == NULL)) {
> +		mutex_unlock(&codec->spdif_mutex);
>  		return true;
> +	}
>  	non_pcm = !!(spdif->status & IEC958_AES0_NONAUDIO);

-	if (WARN_ON(spdif == NULL))
-		return true;
-	non_pcm = !!(spdif->status & IEC958_AES0_NONAUDIO);
+	non_pcm = WARN_ON(!spdif)
+		  ? true
+		  : !!(spdif->status & IEC958_AES0_NONAUDIO);
 	mutex_unlock(&codec->spdif_mutex);
 	return non_pcm;
 }


Would you like to add the tag “Fixes” to the change description?

Regards,
Markus