All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: "Arnd Bergmann" <arnd@arndb.de>
Cc: Mark Brown <broonie@kernel.org>,
	"Jaroslav Kysela" <perex@perex.cz>, <alsa-devel@alsa-project.org>,
	"David Henningsson" <david.henningsson@canonical.com>,
	"Han Lu" <han.lu@intel.com>,
	"Libin Yang" <libin.yang@linux.intel.com>,
	<linux-arm-kernel@lists.infradead.org>,
	"Thierry Reding" <treding@nvidia.com>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] sound: hdmi: avoid dereferencing uninitialized 'jack' pointer
Date: Tue, 16 Feb 2016 16:49:42 +0100	[thread overview]
Message-ID: <s5hbn7g7izt.wl-tiwai@suse.de> (raw)
In-Reply-To: <1455634059-1896914-1-git-send-email-arnd@arndb.de>

On Tue, 16 Feb 2016 15:47:28 +0100,
Arnd Bergmann wrote:
> 
> When CONFIG_SND_JACK is disabled, the hdmi driver tries to create
> a new output jack structure and dereferences it even though
> the pointer is never assigned:
> 
> sound/pci/hda/patch_hdmi.c: In function 'generic_hdmi_build_jack':
> sound/pci/hda/patch_hdmi.c:2534:30: error: 'jack' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> sound/pci/hda/patch_hdmi.c:2526:19: note: 'jack' was declared here
> 
> This uses a compile-time check to avoid calling a nonworking
> snd_jack_new() function, and sets an explicit NULL pointer instead.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 788d441a164c ("ALSA: hda - Use component ops for i915 HDMI/DP audio jack handling")

Thanks for the patch.  But I think it's cleaner to fix Kconfig.

Thinking more of it, maybe splitting jack stuff as a separate module
and does reverse-select to CONFIG_INPUT would be better.  Then its
users can select simply SND_JACK, and everything would fit.

An untested patch is below.  Mark, what do you think?


Takashi

---
diff --git a/sound/core/Kconfig b/sound/core/Kconfig
index a2a1e24becc6..cf4058dde959 100644
--- a/sound/core/Kconfig
+++ b/sound/core/Kconfig
@@ -24,11 +24,9 @@ config SND_RAWMIDI
 config SND_COMPRESS_OFFLOAD
 	tristate
 
-# To be effective this also requires INPUT - users should say:
-#    select SND_JACK if INPUT=y || INPUT=SND
-# to avoid having to force INPUT on.
 config SND_JACK
-	bool
+	tristate
+	select INPUT
 
 config SND_SEQUENCER
 	tristate "Sequencer support"
diff --git a/sound/core/Makefile b/sound/core/Makefile
index 48ab4b8f8279..d16e2b0ba4fb 100644
--- a/sound/core/Makefile
+++ b/sound/core/Makefile
@@ -11,7 +11,8 @@ endif
 snd-$(CONFIG_ISA_DMA_API) += isadma.o
 snd-$(CONFIG_SND_OSSEMUL) += sound_oss.o
 snd-$(CONFIG_SND_VMASTER) += vmaster.o
-snd-$(CONFIG_SND_JACK)	  += ctljack.o jack.o
+
+snd-jack-objs	:= ctljack.o jack.o
 
 snd-pcm-y := pcm.o pcm_native.o pcm_lib.o pcm_misc.o \
 		pcm_memory.o memalloc.o
@@ -41,6 +42,7 @@ obj-$(CONFIG_SND_RTCTIMER)	+= snd-rtctimer.o
 obj-$(CONFIG_SND_PCM)		+= snd-pcm.o
 obj-$(CONFIG_SND_DMAENGINE_PCM)	+= snd-pcm-dmaengine.o
 obj-$(CONFIG_SND_RAWMIDI)	+= snd-rawmidi.o
+obj-$(CONFIG_SND_JACK)		+= snd-jack.o
 
 obj-$(CONFIG_SND_OSSEMUL)	+= oss/
 obj-$(CONFIG_SND_SEQUENCER)	+= seq/
diff --git a/sound/pci/Kconfig b/sound/pci/Kconfig
index 8f6594a7d37f..32151d8c6bb8 100644
--- a/sound/pci/Kconfig
+++ b/sound/pci/Kconfig
@@ -866,7 +866,7 @@ config SND_VIRTUOSO
 	select SND_OXYGEN_LIB
 	select SND_PCM
 	select SND_MPU401_UART
-	select SND_JACK if INPUT=y || INPUT=SND
+	select SND_JACK
 	help
 	  Say Y here to include support for sound cards based on the
 	  Asus AV66/AV100/AV200 chips, i.e., Xonar D1, DX, D2, D2X, DS, DSX,
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
index e94cfd5c69f7..bb02c2d48fd5 100644
--- a/sound/pci/hda/Kconfig
+++ b/sound/pci/hda/Kconfig
@@ -4,7 +4,7 @@ config SND_HDA
 	tristate
 	select SND_PCM
 	select SND_VMASTER
-	select SND_JACK if INPUT=y || INPUT=SND
+	select SND_JACK
 	select SND_HDA_CORE
 
 config SND_HDA_INTEL
diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
index 7ea66ee3653f..182d92efc7c8 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -6,7 +6,7 @@ menuconfig SND_SOC
 	tristate "ALSA for SoC audio support"
 	select SND_PCM
 	select AC97_BUS if SND_SOC_AC97_BUS
-	select SND_JACK if INPUT=y || INPUT=SND
+	select SND_JACK
 	select REGMAP_I2C if I2C
 	select REGMAP_SPI if SPI_MASTER
 	---help---

WARNING: multiple messages have this Message-ID (diff)
From: Takashi Iwai <tiwai@suse.de>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	alsa-devel@alsa-project.org,
	David Henningsson <david.henningsson@canonical.com>,
	Han Lu <han.lu@intel.com>,
	Libin Yang <libin.yang@linux.intel.com>,
	linux-arm-kernel@lists.infradead.org,
	Thierry Reding <treding@nvidia.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sound: hdmi: avoid dereferencing uninitialized 'jack' pointer
Date: Tue, 16 Feb 2016 16:49:42 +0100	[thread overview]
Message-ID: <s5hbn7g7izt.wl-tiwai@suse.de> (raw)
In-Reply-To: <1455634059-1896914-1-git-send-email-arnd@arndb.de>

On Tue, 16 Feb 2016 15:47:28 +0100,
Arnd Bergmann wrote:
> 
> When CONFIG_SND_JACK is disabled, the hdmi driver tries to create
> a new output jack structure and dereferences it even though
> the pointer is never assigned:
> 
> sound/pci/hda/patch_hdmi.c: In function 'generic_hdmi_build_jack':
> sound/pci/hda/patch_hdmi.c:2534:30: error: 'jack' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> sound/pci/hda/patch_hdmi.c:2526:19: note: 'jack' was declared here
> 
> This uses a compile-time check to avoid calling a nonworking
> snd_jack_new() function, and sets an explicit NULL pointer instead.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 788d441a164c ("ALSA: hda - Use component ops for i915 HDMI/DP audio jack handling")

Thanks for the patch.  But I think it's cleaner to fix Kconfig.

Thinking more of it, maybe splitting jack stuff as a separate module
and does reverse-select to CONFIG_INPUT would be better.  Then its
users can select simply SND_JACK, and everything would fit.

An untested patch is below.  Mark, what do you think?


Takashi

---
diff --git a/sound/core/Kconfig b/sound/core/Kconfig
index a2a1e24becc6..cf4058dde959 100644
--- a/sound/core/Kconfig
+++ b/sound/core/Kconfig
@@ -24,11 +24,9 @@ config SND_RAWMIDI
 config SND_COMPRESS_OFFLOAD
 	tristate
 
-# To be effective this also requires INPUT - users should say:
-#    select SND_JACK if INPUT=y || INPUT=SND
-# to avoid having to force INPUT on.
 config SND_JACK
-	bool
+	tristate
+	select INPUT
 
 config SND_SEQUENCER
 	tristate "Sequencer support"
diff --git a/sound/core/Makefile b/sound/core/Makefile
index 48ab4b8f8279..d16e2b0ba4fb 100644
--- a/sound/core/Makefile
+++ b/sound/core/Makefile
@@ -11,7 +11,8 @@ endif
 snd-$(CONFIG_ISA_DMA_API) += isadma.o
 snd-$(CONFIG_SND_OSSEMUL) += sound_oss.o
 snd-$(CONFIG_SND_VMASTER) += vmaster.o
-snd-$(CONFIG_SND_JACK)	  += ctljack.o jack.o
+
+snd-jack-objs	:= ctljack.o jack.o
 
 snd-pcm-y := pcm.o pcm_native.o pcm_lib.o pcm_misc.o \
 		pcm_memory.o memalloc.o
@@ -41,6 +42,7 @@ obj-$(CONFIG_SND_RTCTIMER)	+= snd-rtctimer.o
 obj-$(CONFIG_SND_PCM)		+= snd-pcm.o
 obj-$(CONFIG_SND_DMAENGINE_PCM)	+= snd-pcm-dmaengine.o
 obj-$(CONFIG_SND_RAWMIDI)	+= snd-rawmidi.o
+obj-$(CONFIG_SND_JACK)		+= snd-jack.o
 
 obj-$(CONFIG_SND_OSSEMUL)	+= oss/
 obj-$(CONFIG_SND_SEQUENCER)	+= seq/
diff --git a/sound/pci/Kconfig b/sound/pci/Kconfig
index 8f6594a7d37f..32151d8c6bb8 100644
--- a/sound/pci/Kconfig
+++ b/sound/pci/Kconfig
@@ -866,7 +866,7 @@ config SND_VIRTUOSO
 	select SND_OXYGEN_LIB
 	select SND_PCM
 	select SND_MPU401_UART
-	select SND_JACK if INPUT=y || INPUT=SND
+	select SND_JACK
 	help
 	  Say Y here to include support for sound cards based on the
 	  Asus AV66/AV100/AV200 chips, i.e., Xonar D1, DX, D2, D2X, DS, DSX,
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
index e94cfd5c69f7..bb02c2d48fd5 100644
--- a/sound/pci/hda/Kconfig
+++ b/sound/pci/hda/Kconfig
@@ -4,7 +4,7 @@ config SND_HDA
 	tristate
 	select SND_PCM
 	select SND_VMASTER
-	select SND_JACK if INPUT=y || INPUT=SND
+	select SND_JACK
 	select SND_HDA_CORE
 
 config SND_HDA_INTEL
diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
index 7ea66ee3653f..182d92efc7c8 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -6,7 +6,7 @@ menuconfig SND_SOC
 	tristate "ALSA for SoC audio support"
 	select SND_PCM
 	select AC97_BUS if SND_SOC_AC97_BUS
-	select SND_JACK if INPUT=y || INPUT=SND
+	select SND_JACK
 	select REGMAP_I2C if I2C
 	select REGMAP_SPI if SPI_MASTER
 	---help---

WARNING: multiple messages have this Message-ID (diff)
From: tiwai@suse.de (Takashi Iwai)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] sound: hdmi: avoid dereferencing uninitialized 'jack' pointer
Date: Tue, 16 Feb 2016 16:49:42 +0100	[thread overview]
Message-ID: <s5hbn7g7izt.wl-tiwai@suse.de> (raw)
In-Reply-To: <1455634059-1896914-1-git-send-email-arnd@arndb.de>

On Tue, 16 Feb 2016 15:47:28 +0100,
Arnd Bergmann wrote:
> 
> When CONFIG_SND_JACK is disabled, the hdmi driver tries to create
> a new output jack structure and dereferences it even though
> the pointer is never assigned:
> 
> sound/pci/hda/patch_hdmi.c: In function 'generic_hdmi_build_jack':
> sound/pci/hda/patch_hdmi.c:2534:30: error: 'jack' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> sound/pci/hda/patch_hdmi.c:2526:19: note: 'jack' was declared here
> 
> This uses a compile-time check to avoid calling a nonworking
> snd_jack_new() function, and sets an explicit NULL pointer instead.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 788d441a164c ("ALSA: hda - Use component ops for i915 HDMI/DP audio jack handling")

Thanks for the patch.  But I think it's cleaner to fix Kconfig.

Thinking more of it, maybe splitting jack stuff as a separate module
and does reverse-select to CONFIG_INPUT would be better.  Then its
users can select simply SND_JACK, and everything would fit.

An untested patch is below.  Mark, what do you think?


Takashi

---
diff --git a/sound/core/Kconfig b/sound/core/Kconfig
index a2a1e24becc6..cf4058dde959 100644
--- a/sound/core/Kconfig
+++ b/sound/core/Kconfig
@@ -24,11 +24,9 @@ config SND_RAWMIDI
 config SND_COMPRESS_OFFLOAD
 	tristate
 
-# To be effective this also requires INPUT - users should say:
-#    select SND_JACK if INPUT=y || INPUT=SND
-# to avoid having to force INPUT on.
 config SND_JACK
-	bool
+	tristate
+	select INPUT
 
 config SND_SEQUENCER
 	tristate "Sequencer support"
diff --git a/sound/core/Makefile b/sound/core/Makefile
index 48ab4b8f8279..d16e2b0ba4fb 100644
--- a/sound/core/Makefile
+++ b/sound/core/Makefile
@@ -11,7 +11,8 @@ endif
 snd-$(CONFIG_ISA_DMA_API) += isadma.o
 snd-$(CONFIG_SND_OSSEMUL) += sound_oss.o
 snd-$(CONFIG_SND_VMASTER) += vmaster.o
-snd-$(CONFIG_SND_JACK)	  += ctljack.o jack.o
+
+snd-jack-objs	:= ctljack.o jack.o
 
 snd-pcm-y := pcm.o pcm_native.o pcm_lib.o pcm_misc.o \
 		pcm_memory.o memalloc.o
@@ -41,6 +42,7 @@ obj-$(CONFIG_SND_RTCTIMER)	+= snd-rtctimer.o
 obj-$(CONFIG_SND_PCM)		+= snd-pcm.o
 obj-$(CONFIG_SND_DMAENGINE_PCM)	+= snd-pcm-dmaengine.o
 obj-$(CONFIG_SND_RAWMIDI)	+= snd-rawmidi.o
+obj-$(CONFIG_SND_JACK)		+= snd-jack.o
 
 obj-$(CONFIG_SND_OSSEMUL)	+= oss/
 obj-$(CONFIG_SND_SEQUENCER)	+= seq/
diff --git a/sound/pci/Kconfig b/sound/pci/Kconfig
index 8f6594a7d37f..32151d8c6bb8 100644
--- a/sound/pci/Kconfig
+++ b/sound/pci/Kconfig
@@ -866,7 +866,7 @@ config SND_VIRTUOSO
 	select SND_OXYGEN_LIB
 	select SND_PCM
 	select SND_MPU401_UART
-	select SND_JACK if INPUT=y || INPUT=SND
+	select SND_JACK
 	help
 	  Say Y here to include support for sound cards based on the
 	  Asus AV66/AV100/AV200 chips, i.e., Xonar D1, DX, D2, D2X, DS, DSX,
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
index e94cfd5c69f7..bb02c2d48fd5 100644
--- a/sound/pci/hda/Kconfig
+++ b/sound/pci/hda/Kconfig
@@ -4,7 +4,7 @@ config SND_HDA
 	tristate
 	select SND_PCM
 	select SND_VMASTER
-	select SND_JACK if INPUT=y || INPUT=SND
+	select SND_JACK
 	select SND_HDA_CORE
 
 config SND_HDA_INTEL
diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
index 7ea66ee3653f..182d92efc7c8 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -6,7 +6,7 @@ menuconfig SND_SOC
 	tristate "ALSA for SoC audio support"
 	select SND_PCM
 	select AC97_BUS if SND_SOC_AC97_BUS
-	select SND_JACK if INPUT=y || INPUT=SND
+	select SND_JACK
 	select REGMAP_I2C if I2C
 	select REGMAP_SPI if SPI_MASTER
 	---help---

  reply	other threads:[~2016-02-16 15:49 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-16 14:47 [PATCH] sound: hdmi: avoid dereferencing uninitialized 'jack' pointer Arnd Bergmann
2016-02-16 14:47 ` Arnd Bergmann
2016-02-16 14:47 ` Arnd Bergmann
2016-02-16 15:49 ` Takashi Iwai [this message]
2016-02-16 15:49   ` Takashi Iwai
2016-02-16 15:49   ` Takashi Iwai
2016-02-16 16:09   ` Arnd Bergmann
2016-02-16 16:09     ` Arnd Bergmann
2016-02-16 16:09     ` Arnd Bergmann
2016-02-16 16:18     ` Takashi Iwai
2016-02-16 16:18       ` Takashi Iwai
2016-02-16 16:38       ` Mark Brown
2016-02-16 16:38         ` Mark Brown
2016-02-16 16:43         ` Takashi Iwai
2016-02-16 16:43           ` Takashi Iwai
2016-02-16 16:59         ` Arnd Bergmann
2016-02-16 16:59           ` Arnd Bergmann
2016-02-16 16:59           ` Arnd Bergmann
2016-02-16 17:09           ` Arnd Bergmann
2016-02-16 17:09             ` Arnd Bergmann
2016-02-16 17:09             ` Arnd Bergmann
2016-02-16 17:10           ` Takashi Iwai
2016-02-16 17:10             ` Takashi Iwai
2016-02-16 17:26             ` Arnd Bergmann
2016-02-16 17:26               ` Arnd Bergmann
2016-02-16 17:26               ` Arnd Bergmann
2016-02-16 22:08               ` Arnd Bergmann
2016-02-16 22:08                 ` Arnd Bergmann
2016-02-16 22:08                 ` Arnd Bergmann
2016-02-17  9:03                 ` Takashi Iwai
2016-02-17  9:03                   ` Takashi Iwai
2016-02-17  9:24                   ` [PATCH] ALSA: jack: Allow building the jack layer without input kbuild test robot
2016-02-17  9:24                     ` kbuild test robot
2016-02-17  9:35                   ` [PATCH] sound: hdmi: avoid dereferencing uninitialized 'jack' pointer Takashi Iwai
2016-02-17  9:35                     ` Takashi Iwai
2016-02-17  9:35                     ` Takashi Iwai
2016-02-17  9:40                     ` Arnd Bergmann
2016-02-17  9:40                       ` Arnd Bergmann
2016-02-17  9:40                       ` Arnd Bergmann
2016-02-24 16:18                     ` Arnd Bergmann
2016-02-24 16:18                       ` Arnd Bergmann
2016-02-24 16:18                       ` Arnd Bergmann
2016-02-24 16:25                       ` Takashi Iwai
2016-02-24 16:25                         ` Takashi Iwai
2016-02-24 16:39                         ` Arnd Bergmann
2016-02-24 16:39                           ` Arnd Bergmann
2016-02-24 16:39                           ` Arnd Bergmann
2016-02-26  2:46                       ` Applied "ASoC: trace: fix printing jack name" to the asoc tree Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5hbn7g7izt.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=arnd@arndb.de \
    --cc=broonie@kernel.org \
    --cc=david.henningsson@canonical.com \
    --cc=han.lu@intel.com \
    --cc=libin.yang@linux.intel.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=treding@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.