mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 1/7] ALSA: als100: fix format string overflow warning
@ 2017-07-18 11:48 Arnd Bergmann
  2017-07-18 11:48 ` [PATCH v2 2/7] ALSA: cs423x: " Arnd Bergmann
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Arnd Bergmann @ 2017-07-18 11:48 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai; +Cc: Arnd Bergmann, alsa-devel, linux-kernel

The compiler sees that the format string might overflow for the longname:

sound/isa/als100.c: In function 'snd_als100_pnp_detect':
sound/isa/als100.c:225:27: error: ', dma ' directive writing 6 bytes into a region of size between 0 and 64 [-Werror=format-overflow=]
   sprintf(card->longname, "%s, %s at 0x%lx, irq %d, dma %d",
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sound/isa/als100.c:225:3: note: 'sprintf' output between 24 and 113 bytes into a destination of size 80
   sprintf(card->longname, "%s, %s at 0x%lx, irq %d, dma %d",

Open-coding "shortname" here gets us below the limit, and using
snprintf() is a good idea too.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 sound/isa/als100.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/sound/isa/als100.c b/sound/isa/als100.c
index bc9ea306ee02..6a2c5b48f3ae 100644
--- a/sound/isa/als100.c
+++ b/sound/isa/als100.c
@@ -222,15 +222,16 @@ static int snd_card_als100_probe(int dev,
 	if (pid->driver_data == SB_HW_DT019X) {
 		strcpy(card->driver, "DT-019X");
 		strcpy(card->shortname, "Diamond Tech. DT-019X");
-		sprintf(card->longname, "%s, %s at 0x%lx, irq %d, dma %d",
-			card->shortname, chip->name, chip->port,
-			irq[dev], dma8[dev]);
+		snprintf(card->longname, sizeof(card->longname),
+			 "Diamond Tech. DT-019X, %s at 0x%lx, irq %d, dma %d",
+			 chip->name, chip->port, irq[dev], dma8[dev]);
 	} else {
 		strcpy(card->driver, "ALS100");
 		strcpy(card->shortname, "Avance Logic ALS100");
-		sprintf(card->longname, "%s, %s at 0x%lx, irq %d, dma %d&%d",
-			card->shortname, chip->name, chip->port,
-			irq[dev], dma8[dev], dma16[dev]);
+		snprintf(card->longname, sizeof(card->longname),
+			 "Avance Logic ALS100, %s at 0x%lx, irq %d, dma %d&%d",
+			 chip->name, chip->port, irq[dev], dma8[dev],
+			 dma16[dev]);
 	}
 
 	if ((error = snd_sb16dsp_pcm(chip, 0)) < 0) {
-- 
2.9.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 2/7] ALSA: cs423x: fix format string overflow warning
  2017-07-18 11:48 [PATCH v2 1/7] ALSA: als100: fix format string overflow warning Arnd Bergmann
@ 2017-07-18 11:48 ` Arnd Bergmann
  2017-07-18 11:48 ` [PATCH v2 3/7] ALSA: ad1848: " Arnd Bergmann
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Arnd Bergmann @ 2017-07-18 11:48 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai; +Cc: Arnd Bergmann, alsa-devel, linux-kernel

The snd_pcm name may overflow the card->longname total size:

sound/isa/cs423x/cs4231.c: In function 'snd_cs4231_probe':
sound/isa/cs423x/cs4231.c:115:26: error: ' at 0x' directive writing 6 bytes into a region of size between 1 and 80 [-Werror=format-overflow=] 0x%lx, irq %d, dma %d",
  sprintf(card->longname, "%s at 0x%lx, irq %d, dma %d",
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This changes the driver to use snprintf() so we truncate the string
instead of overflowing into the next field if that happens.

I decided to split out the second format string for the extra
DMA channel to keep the code simpler.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 sound/isa/cs423x/cs4231.c | 18 +++++++++++-------
 sound/isa/cs423x/cs4236.c | 20 +++++++++++---------
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/sound/isa/cs423x/cs4231.c b/sound/isa/cs423x/cs4231.c
index e8edd9017a2f..d90ab9558f7f 100644
--- a/sound/isa/cs423x/cs4231.c
+++ b/sound/isa/cs423x/cs4231.c
@@ -109,13 +109,17 @@ static int snd_cs4231_probe(struct device *dev, unsigned int n)
 	if (error < 0)
 		goto out;
 
-	strcpy(card->driver, "CS4231");
-	strcpy(card->shortname, chip->pcm->name);
-
-	sprintf(card->longname, "%s at 0x%lx, irq %d, dma %d",
-		chip->pcm->name, chip->port, irq[n], dma1[n]);
-	if (dma2[n] >= 0)
-		sprintf(card->longname + strlen(card->longname), "&%d", dma2[n]);
+	strlcpy(card->driver, "CS4231", sizeof(card->driver));
+	strlcpy(card->shortname, chip->pcm->name, sizeof(card->shortname));
+
+	if (dma2[n] < 0)
+		snprintf(card->longname, sizeof(card->longname),
+			 "%s at 0x%lx, irq %d, dma %d",
+			 chip->pcm->name, chip->port, irq[n], dma1[n]);
+	else
+		snprintf(card->longname, sizeof(card->longname),
+			 "%s at 0x%lx, irq %d, dma %d&%d",
+			 chip->pcm->name, chip->port, irq[n], dma1[n], dma2[n]);
 
 	error = snd_wss_mixer(chip);
 	if (error < 0)
diff --git a/sound/isa/cs423x/cs4236.c b/sound/isa/cs423x/cs4236.c
index 1f9a3b2be7a1..4c09756c7353 100644
--- a/sound/isa/cs423x/cs4236.c
+++ b/sound/isa/cs423x/cs4236.c
@@ -419,15 +419,17 @@ static int snd_cs423x_probe(struct snd_card *card, int dev)
 		if (err < 0)
 			return err;
 	}
-	strcpy(card->driver, chip->pcm->name);
-	strcpy(card->shortname, chip->pcm->name);
-	sprintf(card->longname, "%s at 0x%lx, irq %i, dma %i",
-		chip->pcm->name,
-		chip->port,
-		irq[dev],
-		dma1[dev]);
-	if (dma2[dev] >= 0)
-		sprintf(card->longname + strlen(card->longname), "&%d", dma2[dev]);
+	strlcpy(card->driver, chip->pcm->name, sizeof(card->driver));
+	strlcpy(card->shortname, chip->pcm->name, sizeof(card->shortname));
+	if (dma2[dev] < 0)
+		snprintf(card->longname, sizeof(card->longname),
+			 "%s at 0x%lx, irq %i, dma %i",
+			 chip->pcm->name, chip->port, irq[dev], dma1[dev]);
+	else
+		snprintf(card->longname, sizeof(card->longname),
+			 "%s at 0x%lx, irq %i, dma %i&%d",
+			 chip->pcm->name, chip->port, irq[dev], dma1[dev],
+			 dma2[dev]);
 
 	err = snd_wss_timer(chip, 0);
 	if (err < 0)
-- 
2.9.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 3/7] ALSA: ad1848: fix format string overflow warning
  2017-07-18 11:48 [PATCH v2 1/7] ALSA: als100: fix format string overflow warning Arnd Bergmann
  2017-07-18 11:48 ` [PATCH v2 2/7] ALSA: cs423x: " Arnd Bergmann
@ 2017-07-18 11:48 ` Arnd Bergmann
  2017-07-18 11:48 ` [PATCH v2 4/7] ALSA: opti9xx: " Arnd Bergmann
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Arnd Bergmann @ 2017-07-18 11:48 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai; +Cc: Arnd Bergmann, alsa-devel, linux-kernel

The snd_pcm name is too long to fit into the card shortname
or a part of the longname:

sound/isa/ad1848/ad1848.c: In function 'snd_ad1848_probe':
sound/isa/ad1848/ad1848.c:116:26: error: ' at 0x' directive writing 6 bytes into a region of size between 1 and 80 [-Werror=format-overflow=]
  sprintf(card->longname, "%s at 0x%lx, irq %d, dma %d",
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sound/isa/ad1848/ad1848.c:116:2: note: 'sprintf' output between 22 and 128 bytes into a destination of size 80
  sprintf(card->longname, "%s at 0x%lx, irq %d, dma %d",
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   chip->pcm->name, chip->port, irq[n], dma1[n]);
   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This changes the code to use length-checking functions that truncate
if necessary. The "[Thinkpad]" substring is now also part of the
snprintf(), as that could also overflow the buffer.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 sound/isa/ad1848/ad1848.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/sound/isa/ad1848/ad1848.c b/sound/isa/ad1848/ad1848.c
index e739b1c85c25..7c8e92f62f3b 100644
--- a/sound/isa/ad1848/ad1848.c
+++ b/sound/isa/ad1848/ad1848.c
@@ -110,13 +110,17 @@ static int snd_ad1848_probe(struct device *dev, unsigned int n)
 	if (error < 0)
 		goto out;
 
-	strcpy(card->driver, "AD1848");
-	strcpy(card->shortname, chip->pcm->name);
-
-	sprintf(card->longname, "%s at 0x%lx, irq %d, dma %d",
-		chip->pcm->name, chip->port, irq[n], dma1[n]);
-	if (thinkpad[n])
-		strcat(card->longname, " [Thinkpad]");
+	strlcpy(card->driver, "AD1848", sizeof(card->driver));
+	strlcpy(card->shortname, chip->pcm->name, sizeof(card->shortname));
+
+	if (!thinkpad[n])
+		snprintf(card->longname, sizeof(card->longname),
+			 "%s at 0x%lx, irq %d, dma %d",
+			 chip->pcm->name, chip->port, irq[n], dma1[n]);
+	else
+		snprintf(card->longname, sizeof(card->longname),
+			 "%s at 0x%lx, irq %d, dma %d [Thinkpad]",
+			 chip->pcm->name, chip->port, irq[n], dma1[n]);
 
 	error = snd_card_register(card);
 	if (error < 0)
-- 
2.9.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 4/7] ALSA: opti9xx: fix format string overflow warning
  2017-07-18 11:48 [PATCH v2 1/7] ALSA: als100: fix format string overflow warning Arnd Bergmann
  2017-07-18 11:48 ` [PATCH v2 2/7] ALSA: cs423x: " Arnd Bergmann
  2017-07-18 11:48 ` [PATCH v2 3/7] ALSA: ad1848: " Arnd Bergmann
@ 2017-07-18 11:48 ` Arnd Bergmann
  2017-07-18 11:48 ` [PATCH v2 5/7] ALSA: mixart: fix " Arnd Bergmann
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Arnd Bergmann @ 2017-07-18 11:48 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, Arnd Bergmann; +Cc: alsa-devel, linux-kernel

We pass a long name from "codec->pcm->name" into the longname
string of the same length:

sound/isa/opti9xx/miro.c: In function 'snd_miro_probe':
sound/isa/opti9xx/miro.c:1356:39: error: '%s' directive writing up to 79 bytes into a region of size between 35 and 72 [-Werror=format-overflow=]
  sprintf(card->longname, "%s: OPTi%s, %s at 0x%lx, irq %d, dma %d&%d",
                                       ^~
sound/isa/opti9xx/miro.c:1356:26: note: using the range [0, 4294967295] for directive argument
  sprintf(card->longname, "%s: OPTi%s, %s at 0x%lx, irq %d, dma %d&%d",
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sound/isa/opti9xx/miro.c:1356:2: note: 'sprintf' output between 32 and 185 bytes into a destination of size 80
  sprintf(card->longname, "%s: OPTi%s, %s at 0x%lx, irq %d, dma %d&%d",

There is no easy way to avoid the theoretical overflow in this case,
but using snprintf() will turn it into a harmless truncation.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 sound/isa/opti9xx/miro.c           |  7 ++++---
 sound/isa/opti9xx/opti92x-ad1848.c | 14 ++++++++------
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/sound/isa/opti9xx/miro.c b/sound/isa/opti9xx/miro.c
index bcbff56f060d..1c5a12fa8be3 100644
--- a/sound/isa/opti9xx/miro.c
+++ b/sound/isa/opti9xx/miro.c
@@ -1353,9 +1353,10 @@ static int snd_miro_probe(struct snd_card *card)
 	}
 
 	strcpy(card->driver, "miro");
-	sprintf(card->longname, "%s: OPTi%s, %s at 0x%lx, irq %d, dma %d&%d",
-		card->shortname, miro->name, codec->pcm->name,
-		miro->wss_base + 4, miro->irq, miro->dma1, miro->dma2);
+	snprintf(card->longname, sizeof(card->longname),
+		 "%s: OPTi%s, %s at 0x%lx, irq %d, dma %d&%d",
+		 card->shortname, miro->name, codec->pcm->name,
+		 miro->wss_base + 4, miro->irq, miro->dma1, miro->dma2);
 
 	if (mpu_port <= 0 || mpu_port == SNDRV_AUTO_PORT)
 		rmidi = NULL;
diff --git a/sound/isa/opti9xx/opti92x-ad1848.c b/sound/isa/opti9xx/opti92x-ad1848.c
index ceddb392b1e3..56dc581c57d5 100644
--- a/sound/isa/opti9xx/opti92x-ad1848.c
+++ b/sound/isa/opti9xx/opti92x-ad1848.c
@@ -879,13 +879,15 @@ static int snd_opti9xx_probe(struct snd_card *card)
 	strcpy(card->driver, chip->name);
 	sprintf(card->shortname, "OPTi %s", card->driver);
 #if defined(CS4231) || defined(OPTi93X)
-	sprintf(card->longname, "%s, %s at 0x%lx, irq %d, dma %d&%d",
-		card->shortname, codec->pcm->name,
-		chip->wss_base + 4, irq, dma1, xdma2);
+	snprintf(card->longname, sizeof(card->longname),
+		 "%s, %s at 0x%lx, irq %d, dma %d&%d",
+		 card->shortname, codec->pcm->name,
+		 chip->wss_base + 4, irq, dma1, xdma2);
 #else
-	sprintf(card->longname, "%s, %s at 0x%lx, irq %d, dma %d",
-		card->shortname, codec->pcm->name, chip->wss_base + 4, irq,
-		dma1);
+	snprintf(card->longname, sizeof(card->longname),
+		 "%s, %s at 0x%lx, irq %d, dma %d",
+		 card->shortname, codec->pcm->name, chip->wss_base + 4, irq,
+		 dma1);
 #endif	/* CS4231 || OPTi93X */
 
 	if (mpu_port <= 0 || mpu_port == SNDRV_AUTO_PORT)
-- 
2.9.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 5/7] ALSA: mixart: fix string overflow warning
  2017-07-18 11:48 [PATCH v2 1/7] ALSA: als100: fix format string overflow warning Arnd Bergmann
                   ` (2 preceding siblings ...)
  2017-07-18 11:48 ` [PATCH v2 4/7] ALSA: opti9xx: " Arnd Bergmann
@ 2017-07-18 11:48 ` Arnd Bergmann
  2017-07-18 11:48 ` [PATCH v2 6/7] ALSA: rme9652: fix format overflow warnings Arnd Bergmann
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Arnd Bergmann @ 2017-07-18 11:48 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai; +Cc: Arnd Bergmann, alsa-devel, linux-kernel

Using a temporary string produces warnings about a possible overflow
in sprintf:

sound/pci/mixart/mixart.c: In function 'snd_mixart_probe':
sound/pci/mixart/mixart.c:1353:28: error: ' [PCM #' directive writing 7 bytes into a region of size between 1 and 32 [-Werror=format-overflow=]
   sprintf(card->shortname, "%s [PCM #%d]", mgr->shortname, i);
                            ^~~~~~~~~~~~~~
sound/pci/mixart/mixart.c:1353:28: note: using the range [-2147483648, 2147483647] for directive argument
sound/pci/mixart/mixart.c:1353:3: note: 'sprintf' output between 10 and 51 bytes into a destination of size 32
   sprintf(card->shortname, "%s [PCM #%d]", mgr->shortname, i);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sound/pci/mixart/mixart.c:1354:27: error: ' [PCM #' directive writing 7 bytes into a region of size between 1 and 80 [-Werror=format-overflow=]
   sprintf(card->longname, "%s [PCM #%d]", mgr->longname, i);
                           ^~~~~~~~~~~~~~
sound/pci/mixart/mixart.c:1354:27: note: using the range [-2147483648, 2147483647] for directive argument
sound/pci/mixart/mixart.c:1354:3: note: 'sprintf' output between 10 and 99 bytes into a destination of size 80

Skipping the intermediate, we can get gcc to see that it is in fact
safe here.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 sound/pci/mixart/mixart.c | 10 +++++-----
 sound/pci/mixart/mixart.h |  4 ----
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/sound/pci/mixart/mixart.c b/sound/pci/mixart/mixart.c
index 80d439944cb5..6d7fbf30618b 100644
--- a/sound/pci/mixart/mixart.c
+++ b/sound/pci/mixart/mixart.c
@@ -1313,9 +1313,6 @@ static int snd_mixart_probe(struct pci_dev *pci,
 	}
 	mgr->irq = pci->irq;
 
-	sprintf(mgr->shortname, "Digigram miXart");
-	sprintf(mgr->longname, "%s at 0x%lx & 0x%lx, irq %i", mgr->shortname, mgr->mem[0].phys, mgr->mem[1].phys, mgr->irq);
-
 	/* init mailbox  */
 	mgr->msg_fifo_readptr = 0;
 	mgr->msg_fifo_writeptr = 0;
@@ -1350,8 +1347,11 @@ static int snd_mixart_probe(struct pci_dev *pci,
 		}
 
 		strcpy(card->driver, CARD_NAME);
-		sprintf(card->shortname, "%s [PCM #%d]", mgr->shortname, i);
-		sprintf(card->longname, "%s [PCM #%d]", mgr->longname, i);
+		snprintf(card->shortname, sizeof(card->shortname),
+			 "Digigram miXart [PCM #%d]", i);
+		snprintf(card->longname, sizeof(card->longname),
+			"Digigram miXart at 0x%lx & 0x%lx, irq %i [PCM #%d]",
+			mgr->mem[0].phys, mgr->mem[1].phys, mgr->irq, i);
 
 		if ((err = snd_mixart_create(mgr, card, i)) < 0) {
 			snd_card_free(card);
diff --git a/sound/pci/mixart/mixart.h b/sound/pci/mixart/mixart.h
index 426743871540..69b3ece099ad 100644
--- a/sound/pci/mixart/mixart.h
+++ b/sound/pci/mixart/mixart.h
@@ -74,10 +74,6 @@ struct mixart_mgr {
 	/* memory-maps */
 	struct mem_area mem[2];
 
-	/* share the name */
-	char shortname[32];         /* short name of this soundcard */
-	char longname[80];          /* name of this soundcard */
-
 	/* one and only blocking message or notification may be pending  */
 	u32 pending_event;
 	wait_queue_head_t msg_sleep;
-- 
2.9.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 6/7] ALSA: rme9652: fix format overflow warnings
  2017-07-18 11:48 [PATCH v2 1/7] ALSA: als100: fix format string overflow warning Arnd Bergmann
                   ` (3 preceding siblings ...)
  2017-07-18 11:48 ` [PATCH v2 5/7] ALSA: mixart: fix " Arnd Bergmann
@ 2017-07-18 11:48 ` Arnd Bergmann
  2017-07-18 11:48 ` [PATCH v2 7/7] ALSA: pcxhr: fix string " Arnd Bergmann
  2017-07-18 16:00 ` [PATCH v2 1/7] ALSA: als100: fix format string overflow warning Takashi Iwai
  6 siblings, 0 replies; 8+ messages in thread
From: Arnd Bergmann @ 2017-07-18 11:48 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, Arnd Bergmann
  Cc: Takashi Sakamoto, Julia Lawall, Colin Ian King, alsa-devel, linux-kernel

gcc-7 warns about a possible sprintf format string overflow with a
temporary buffer that is used to print from another buffer of the same
size:

sound/pci/rme9652/hdspm.c: In function 'snd_hdspm_create_alsa_devices':
sound/pci/rme9652/hdspm.c:2123:17: error: ' MIDIoverMADI' directive writing 13 bytes into a region of size between 1 and 32 [-Werror=format-overflow=]

This extends the temporary buffer to twice the size, and changes
the code to use the safer snprintf() across the entire file.
The longer buffer is still necessary to avoid a format-truncation
warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 sound/pci/rme9652/hdspm.c | 48 +++++++++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/sound/pci/rme9652/hdspm.c b/sound/pci/rme9652/hdspm.c
index 254c3d040118..2a3a916e5d15 100644
--- a/sound/pci/rme9652/hdspm.c
+++ b/sound/pci/rme9652/hdspm.c
@@ -2061,7 +2061,7 @@ static int snd_hdspm_create_midi(struct snd_card *card,
 				 struct hdspm *hdspm, int id)
 {
 	int err;
-	char buf[32];
+	char buf[64];
 
 	hdspm->midi[id].id = id;
 	hdspm->midi[id].hdspm = hdspm;
@@ -2120,19 +2120,23 @@ static int snd_hdspm_create_midi(struct snd_card *card,
 	if ((id < 2) || ((2 == id) && ((MADI == hdspm->io_type) ||
 					(MADIface == hdspm->io_type)))) {
 		if ((id == 0) && (MADIface == hdspm->io_type)) {
-			sprintf(buf, "%s MIDIoverMADI", card->shortname);
+			snprintf(buf, sizeof(buf), "%s MIDIoverMADI",
+				 card->shortname);
 		} else if ((id == 2) && (MADI == hdspm->io_type)) {
-			sprintf(buf, "%s MIDIoverMADI", card->shortname);
+			snprintf(buf, sizeof(buf), "%s MIDIoverMADI",
+				 card->shortname);
 		} else {
-			sprintf(buf, "%s MIDI %d", card->shortname, id+1);
+			snprintf(buf, sizeof(buf), "%s MIDI %d",
+				 card->shortname, id+1);
 		}
 		err = snd_rawmidi_new(card, buf, id, 1, 1,
 				&hdspm->midi[id].rmidi);
 		if (err < 0)
 			return err;
 
-		sprintf(hdspm->midi[id].rmidi->name, "%s MIDI %d",
-				card->id, id+1);
+		snprintf(hdspm->midi[id].rmidi->name,
+			 sizeof(hdspm->midi[id].rmidi->name),
+			 "%s MIDI %d", card->id, id+1);
 		hdspm->midi[id].rmidi->private_data = &hdspm->midi[id];
 
 		snd_rawmidi_set_ops(hdspm->midi[id].rmidi,
@@ -2148,14 +2152,16 @@ static int snd_hdspm_create_midi(struct snd_card *card,
 			SNDRV_RAWMIDI_INFO_DUPLEX;
 	} else {
 		/* TCO MTC, read only */
-		sprintf(buf, "%s MTC %d", card->shortname, id+1);
+		snprintf(buf, sizeof(buf), "%s MTC %d",
+			 card->shortname, id+1);
 		err = snd_rawmidi_new(card, buf, id, 1, 1,
 				&hdspm->midi[id].rmidi);
 		if (err < 0)
 			return err;
 
-		sprintf(hdspm->midi[id].rmidi->name,
-				"%s MTC %d", card->id, id+1);
+		snprintf(hdspm->midi[id].rmidi->name,
+			 sizeof(hdspm->midi[id].rmidi->name),
+			 "%s MTC %d", card->id, id+1);
 		hdspm->midi[id].rmidi->private_data = &hdspm->midi[id];
 
 		snd_rawmidi_set_ops(hdspm->midi[id].rmidi,
@@ -6869,7 +6875,8 @@ static int snd_hdspm_create(struct snd_card *card,
 		 * when running with multiple cards.
 		 */
 		if (NULL == id[hdspm->dev] && hdspm->serial != 0xFFFFFF) {
-			sprintf(card->id, "HDSPMx%06x", hdspm->serial);
+			snprintf(card->id, sizeof(card->id),
+				 "HDSPMx%06x", hdspm->serial);
 			snd_card_set_id(card, card->id);
 		}
 	}
@@ -6954,17 +6961,18 @@ static int snd_hdspm_probe(struct pci_dev *pci,
 	}
 
 	if (hdspm->io_type != MADIface) {
-		sprintf(card->shortname, "%s_%x",
-			hdspm->card_name,
-			hdspm->serial);
-		sprintf(card->longname, "%s S/N 0x%x at 0x%lx, irq %d",
-			hdspm->card_name,
-			hdspm->serial,
-			hdspm->port, hdspm->irq);
+		snprintf(card->shortname, sizeof(card->shortname), "%s_%x",
+			hdspm->card_name, hdspm->serial);
+		snprintf(card->longname, sizeof(card->longname),
+			 "%s S/N 0x%x at 0x%lx, irq %d",
+			 hdspm->card_name, hdspm->serial,
+			 hdspm->port, hdspm->irq);
 	} else {
-		sprintf(card->shortname, "%s", hdspm->card_name);
-		sprintf(card->longname, "%s at 0x%lx, irq %d",
-				hdspm->card_name, hdspm->port, hdspm->irq);
+		snprintf(card->shortname, sizeof(card->shortname), "%s",
+			 hdspm->card_name);
+		snprintf(card->longname, sizeof(card->longname),
+			 "%s at 0x%lx, irq %d",
+			 hdspm->card_name, hdspm->port, hdspm->irq);
 	}
 
 	err = snd_card_register(card);
-- 
2.9.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 7/7] ALSA: pcxhr: fix string overflow warnings
  2017-07-18 11:48 [PATCH v2 1/7] ALSA: als100: fix format string overflow warning Arnd Bergmann
                   ` (4 preceding siblings ...)
  2017-07-18 11:48 ` [PATCH v2 6/7] ALSA: rme9652: fix format overflow warnings Arnd Bergmann
@ 2017-07-18 11:48 ` Arnd Bergmann
  2017-07-18 16:00 ` [PATCH v2 1/7] ALSA: als100: fix format string overflow warning Takashi Iwai
  6 siblings, 0 replies; 8+ messages in thread
From: Arnd Bergmann @ 2017-07-18 11:48 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai
  Cc: Arnd Bergmann, Julia Lawall, alsa-devel, linux-kernel

With gcc-7, we get a warning about a possible string overflow:

sound/pci/pcxhr/pcxhr.c: In function 'pcxhr_probe':
sound/pci/pcxhr/pcxhr.c:1647:28: error: ' [PCM #' directive writing 7 bytes into a region of size between 1 and 32 [-Werror=format-overflow=]

The shortname can simply be removed, and the longname can
be changed into a shorter "name" string that is used in three
places. Making it a little shorter (40 bytes) avoids the risk of
overflowing completely, but I also use snprintf() here for
extra clarity.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 sound/pci/pcxhr/pcxhr.c | 19 ++++++++++---------
 sound/pci/pcxhr/pcxhr.h |  3 +--
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/sound/pci/pcxhr/pcxhr.c b/sound/pci/pcxhr/pcxhr.c
index bb7eee9d0c2b..fa919f52e225 100644
--- a/sound/pci/pcxhr/pcxhr.c
+++ b/sound/pci/pcxhr/pcxhr.c
@@ -1165,7 +1165,7 @@ int pcxhr_create_pcm(struct snd_pcxhr *chip)
 	struct snd_pcm *pcm;
 	char name[32];
 
-	sprintf(name, "pcxhr %d", chip->chip_idx);
+	snprintf(name, sizeof(name), "pcxhr %d", chip->chip_idx);
 	if ((err = snd_pcm_new(chip->card, name, 0,
 			       chip->nb_streams_play,
 			       chip->nb_streams_capt, &pcm)) < 0) {
@@ -1252,7 +1252,7 @@ static void pcxhr_proc_info(struct snd_info_entry *entry,
 	struct snd_pcxhr *chip = entry->private_data;
 	struct pcxhr_mgr *mgr = chip->mgr;
 
-	snd_iprintf(buffer, "\n%s\n", mgr->longname);
+	snd_iprintf(buffer, "\n%s\n", mgr->name);
 
 	/* stats available when embedded DSP is running */
 	if (mgr->dsp_loaded & (1 << PCXHR_FIRMWARE_DSP_MAIN_INDEX)) {
@@ -1339,7 +1339,7 @@ static void pcxhr_proc_sync(struct snd_info_entry *entry,
 		max_clock = PCXHR_CLOCK_TYPE_MAX;
 	}
 
-	snd_iprintf(buffer, "\n%s\n", mgr->longname);
+	snd_iprintf(buffer, "\n%s\n", mgr->name);
 	snd_iprintf(buffer, "Current Sample Clock\t: %s\n",
 		    texts[mgr->cur_clock_type]);
 	snd_iprintf(buffer, "Current Sample Rate\t= %d\n",
@@ -1597,10 +1597,9 @@ static int pcxhr_probe(struct pci_dev *pci,
 	}
 	mgr->irq = pci->irq;
 
-	sprintf(mgr->shortname, "Digigram %s", card_name);
-	sprintf(mgr->longname, "%s at 0x%lx & 0x%lx, 0x%lx irq %i",
-		mgr->shortname,
-		mgr->port[0], mgr->port[1], mgr->port[2], mgr->irq);
+	snprintf(mgr->name, sizeof(mgr->name),
+		 "Digigram at 0x%lx & 0x%lx, 0x%lx irq %i",
+		 mgr->port[0], mgr->port[1], mgr->port[2], mgr->irq);
 
 	/* ISR lock  */
 	mutex_init(&mgr->lock);
@@ -1644,8 +1643,10 @@ static int pcxhr_probe(struct pci_dev *pci,
 		}
 
 		strcpy(card->driver, DRIVER_NAME);
-		sprintf(card->shortname, "%s [PCM #%d]", mgr->shortname, i);
-		sprintf(card->longname, "%s [PCM #%d]", mgr->longname, i);
+		snprintf(card->shortname, sizeof(card->shortname),
+			 "Digigram [PCM #%d]", i);
+		snprintf(card->longname, sizeof(card->longname),
+			 "%s [PCM #%d]", mgr->name, i);
 
 		if ((err = pcxhr_create(mgr, card, i)) < 0) {
 			snd_card_free(card);
diff --git a/sound/pci/pcxhr/pcxhr.h b/sound/pci/pcxhr/pcxhr.h
index 9e39e509a3ef..d799cbd37301 100644
--- a/sound/pci/pcxhr/pcxhr.h
+++ b/sound/pci/pcxhr/pcxhr.h
@@ -75,8 +75,7 @@ struct pcxhr_mgr {
 	unsigned long port[3];
 
 	/* share the name */
-	char shortname[32];		/* short name of this soundcard */
-	char longname[96];		/* name of this soundcard */
+	char name[40];			/* name of this soundcard */
 
 	struct pcxhr_rmh *prmh;
 
-- 
2.9.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/7] ALSA: als100: fix format string overflow warning
  2017-07-18 11:48 [PATCH v2 1/7] ALSA: als100: fix format string overflow warning Arnd Bergmann
                   ` (5 preceding siblings ...)
  2017-07-18 11:48 ` [PATCH v2 7/7] ALSA: pcxhr: fix string " Arnd Bergmann
@ 2017-07-18 16:00 ` Takashi Iwai
  6 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2017-07-18 16:00 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Jaroslav Kysela, alsa-devel, linux-kernel

On Tue, 18 Jul 2017 13:48:04 +0200,
Arnd Bergmann wrote:
> 
> The compiler sees that the format string might overflow for the longname:
> 
> sound/isa/als100.c: In function 'snd_als100_pnp_detect':
> sound/isa/als100.c:225:27: error: ', dma ' directive writing 6 bytes into a region of size between 0 and 64 [-Werror=format-overflow=]
>    sprintf(card->longname, "%s, %s at 0x%lx, irq %d, dma %d",
>                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> sound/isa/als100.c:225:3: note: 'sprintf' output between 24 and 113 bytes into a destination of size 80
>    sprintf(card->longname, "%s, %s at 0x%lx, irq %d, dma %d",
> 
> Open-coding "shortname" here gets us below the limit, and using
> snprintf() is a good idea too.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Thanks, applied all 7 patches now to for-next branch.


Takashi

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-07-18 16:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-18 11:48 [PATCH v2 1/7] ALSA: als100: fix format string overflow warning Arnd Bergmann
2017-07-18 11:48 ` [PATCH v2 2/7] ALSA: cs423x: " Arnd Bergmann
2017-07-18 11:48 ` [PATCH v2 3/7] ALSA: ad1848: " Arnd Bergmann
2017-07-18 11:48 ` [PATCH v2 4/7] ALSA: opti9xx: " Arnd Bergmann
2017-07-18 11:48 ` [PATCH v2 5/7] ALSA: mixart: fix " Arnd Bergmann
2017-07-18 11:48 ` [PATCH v2 6/7] ALSA: rme9652: fix format overflow warnings Arnd Bergmann
2017-07-18 11:48 ` [PATCH v2 7/7] ALSA: pcxhr: fix string " Arnd Bergmann
2017-07-18 16:00 ` [PATCH v2 1/7] ALSA: als100: fix format string overflow warning Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®