From: Takashi Iwai <tiwai@suse.de> To: "Arnd Bergmann" <arnd@arndb.de> Cc: "Jaroslav Kysela" <perex@perex.cz>, <linux-kernel@vger.kernel.org>, <alsa-devel@alsa-project.org>, "David S . Miller" <davem@davemloft.net>, "Geliang Tang" <geliangtang@gmail.com>, <x86@kernel.org>, <akpm@linux-foundation.org>, "Linus Torvalds" <torvalds@linux-foundation.org>, "James E . J . Bottomley" <jejb@linux.vnet.ibm.com>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Martin K . Petersen" <martin.petersen@oracle.com>, "Guenter Roeck" <linux@roeck-us.net>, "Takashi Sakamoto" <o-takashi@sakamocchi.jp>, <linux-scsi@vger.kernel.org>, <netdev@vger.kernel.org> Subject: Re: [PATCH 20/22] sound: pci: avoid string overflow warnings Date: Fri, 14 Jul 2017 14:28:43 +0200 [thread overview] Message-ID: <s5hinivfbic.wl-tiwai@suse.de> (raw) In-Reply-To: <20170714120720.906842-21-arnd@arndb.de> On Fri, 14 Jul 2017 14:07:12 +0200, Arnd Bergmann wrote: > > With gcc-7, we get various warnings about a possible string overflow: > > 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=] > 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=] > 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 > > I have checked these all and found that the driver-private > shortname strings for mixart and pcxhr are longer than necessary, > and making them shorter will be safe while also making it clear > that no overflow can happen when they get passed as a substring > into the card shortname. > > For hdspm, we have a local buffer of the same size as its substring. > In this case, making the buffer a little longer is safe as the > functions that take it as an argument all use length checking and > the strings we pass into it are actually short enough. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Thanks for the patch. I have seen it but ignored, so far, as not sure which action is the best. An alternative solution is to use snprintf() blindly, for example. For mixart, it's even better to drop mgr->shortname[] and longname[] assignment. The shortname is the fixed string, and the longname is used only at copying to card->longname, so we can create a string there from the scratch. Takashi > --- > sound/pci/mixart/mixart.h | 4 ++-- > sound/pci/pcxhr/pcxhr.h | 4 ++-- > sound/pci/rme9652/hdspm.c | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/sound/pci/mixart/mixart.h b/sound/pci/mixart/mixart.h > index 426743871540..c8309e327663 100644 > --- a/sound/pci/mixart/mixart.h > +++ b/sound/pci/mixart/mixart.h > @@ -75,8 +75,8 @@ struct mixart_mgr { > struct mem_area mem[2]; > > /* share the name */ > - char shortname[32]; /* short name of this soundcard */ > - char longname[80]; /* name of this soundcard */ > + char shortname[16]; /* short name of this soundcard */ > + char longname[40]; /* name of this soundcard */ > > /* one and only blocking message or notification may be pending */ > u32 pending_event; > diff --git a/sound/pci/pcxhr/pcxhr.h b/sound/pci/pcxhr/pcxhr.h > index 9e39e509a3ef..4909a43ce3d9 100644 > --- a/sound/pci/pcxhr/pcxhr.h > +++ b/sound/pci/pcxhr/pcxhr.h > @@ -75,8 +75,8 @@ 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 shortname[16]; /* short name of this soundcard */ > + char longname[40]; /* name of this soundcard */ > > struct pcxhr_rmh *prmh; > > diff --git a/sound/pci/rme9652/hdspm.c b/sound/pci/rme9652/hdspm.c > index 254c3d040118..a1cbf5938a0e 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; > -- > 2.9.0 > >
WARNING: multiple messages have this Message-ID (diff)
From: Takashi Iwai <tiwai@suse.de> To: "Arnd Bergmann" <arnd@arndb.de> Cc: alsa-devel@alsa-project.org, "James E . J . Bottomley" <jejb@linux.vnet.ibm.com>, linux-scsi@vger.kernel.org, "Martin K . Petersen" <martin.petersen@oracle.com>, Geliang Tang <geliangtang@gmail.com>, x86@kernel.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, akpm@linux-foundation.org, Takashi Sakamoto <o-takashi@sakamocchi.jp>, Linus Torvalds <torvalds@linux-foundation.org>, "David S . Miller" <davem@davemloft.net>, Guenter Roeck <linux@roeck-us.net> Subject: Re: [PATCH 20/22] sound: pci: avoid string overflow warnings Date: Fri, 14 Jul 2017 14:28:43 +0200 [thread overview] Message-ID: <s5hinivfbic.wl-tiwai@suse.de> (raw) In-Reply-To: <20170714120720.906842-21-arnd@arndb.de> On Fri, 14 Jul 2017 14:07:12 +0200, Arnd Bergmann wrote: > > With gcc-7, we get various warnings about a possible string overflow: > > 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=] > 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=] > 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 > > I have checked these all and found that the driver-private > shortname strings for mixart and pcxhr are longer than necessary, > and making them shorter will be safe while also making it clear > that no overflow can happen when they get passed as a substring > into the card shortname. > > For hdspm, we have a local buffer of the same size as its substring. > In this case, making the buffer a little longer is safe as the > functions that take it as an argument all use length checking and > the strings we pass into it are actually short enough. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Thanks for the patch. I have seen it but ignored, so far, as not sure which action is the best. An alternative solution is to use snprintf() blindly, for example. For mixart, it's even better to drop mgr->shortname[] and longname[] assignment. The shortname is the fixed string, and the longname is used only at copying to card->longname, so we can create a string there from the scratch. Takashi > --- > sound/pci/mixart/mixart.h | 4 ++-- > sound/pci/pcxhr/pcxhr.h | 4 ++-- > sound/pci/rme9652/hdspm.c | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/sound/pci/mixart/mixart.h b/sound/pci/mixart/mixart.h > index 426743871540..c8309e327663 100644 > --- a/sound/pci/mixart/mixart.h > +++ b/sound/pci/mixart/mixart.h > @@ -75,8 +75,8 @@ struct mixart_mgr { > struct mem_area mem[2]; > > /* share the name */ > - char shortname[32]; /* short name of this soundcard */ > - char longname[80]; /* name of this soundcard */ > + char shortname[16]; /* short name of this soundcard */ > + char longname[40]; /* name of this soundcard */ > > /* one and only blocking message or notification may be pending */ > u32 pending_event; > diff --git a/sound/pci/pcxhr/pcxhr.h b/sound/pci/pcxhr/pcxhr.h > index 9e39e509a3ef..4909a43ce3d9 100644 > --- a/sound/pci/pcxhr/pcxhr.h > +++ b/sound/pci/pcxhr/pcxhr.h > @@ -75,8 +75,8 @@ 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 shortname[16]; /* short name of this soundcard */ > + char longname[40]; /* name of this soundcard */ > > struct pcxhr_rmh *prmh; > > diff --git a/sound/pci/rme9652/hdspm.c b/sound/pci/rme9652/hdspm.c > index 254c3d040118..a1cbf5938a0e 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; > -- > 2.9.0 > >
next prev parent reply other threads:[~2017-07-14 12:28 UTC|newest] Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann 2017-07-14 12:06 ` [PATCH 01/22] kbuild: disable -Wformat-truncation warnings by default Arnd Bergmann 2017-07-14 12:06 ` [PATCH 02/22] scsi: megaraid: fix format-overflow warning Arnd Bergmann 2017-07-14 12:06 ` [PATCH 03/22] scsi: mpt3sas: fix format overflow warning Arnd Bergmann 2017-07-14 12:06 ` Arnd Bergmann 2017-07-14 12:06 ` [PATCH 04/22] scsi: fusion: fix string " Arnd Bergmann 2017-07-14 12:06 ` Arnd Bergmann 2017-07-17 9:17 ` David Laight 2017-07-17 9:17 ` David Laight 2017-07-17 12:00 ` Arnd Bergmann 2017-07-17 12:00 ` Arnd Bergmann 2017-07-17 12:00 ` Arnd Bergmann 2017-07-14 12:06 ` [PATCH 05/22] scsi: gdth: avoid buffer " Arnd Bergmann 2017-07-14 12:06 ` [PATCH 06/22] scsi: fnic: fix format string " Arnd Bergmann 2017-07-14 12:06 ` [PATCH 07/22] scsi: gdth: increase the procfs event buffer size Arnd Bergmann 2017-07-14 12:07 ` [PATCH 08/22] isdn: divert: fix sprintf buffer overflow warning Arnd Bergmann 2017-07-14 16:03 ` David Miller 2017-07-14 12:07 ` [PATCH 09/22] net: niu: fix format string overflow warning: Arnd Bergmann 2017-07-14 16:03 ` David Miller 2017-07-14 12:07 ` [PATCH 10/22] bnx2x: fix format overflow warning Arnd Bergmann 2017-07-14 16:03 ` David Miller 2017-07-14 12:07 ` [PATCH 11/22] net: thunder_bgx: avoid format string " Arnd Bergmann 2017-07-14 12:07 ` Arnd Bergmann 2017-07-14 12:07 ` Arnd Bergmann 2017-07-14 12:33 ` Robin Murphy 2017-07-14 12:33 ` Robin Murphy 2017-07-14 16:03 ` David Miller 2017-07-14 16:03 ` David Miller 2017-07-14 12:07 ` [PATCH 12/22] vmxnet3: avoid format strint " Arnd Bergmann 2017-07-14 16:04 ` David Miller 2017-07-14 12:07 ` [PATCH 13/22] liquidio: fix possible eeprom format string overflow Arnd Bergmann 2017-07-14 16:04 ` David Miller 2017-07-14 22:40 ` Burla, Satananda 2017-07-14 22:40 ` Burla, Satananda 2017-07-14 22:40 ` Burla, Satananda 2017-07-14 12:07 ` [PATCH 14/22] [media] usbvision-i2c: fix format overflow warning Arnd Bergmann 2017-07-17 12:53 ` Hans Verkuil 2017-07-17 12:57 ` Arnd Bergmann 2017-07-17 12:59 ` Hans Verkuil 2017-07-14 12:07 ` [PATCH 15/22] hwmon: applesmc: fix format string overflow Arnd Bergmann 2017-07-14 14:06 ` Guenter Roeck 2017-07-14 12:07 ` [PATCH 16/22] x86: intel-mid: fix a format string overflow warning Arnd Bergmann 2017-07-14 12:07 ` [PATCH 17/22] platform/x86: alienware-wmi: fix " Arnd Bergmann 2017-07-14 18:30 ` Mario.Limonciello 2017-07-14 18:30 ` Mario.Limonciello 2017-07-14 19:18 ` Andy Shevchenko 2017-07-14 19:37 ` Arnd Bergmann 2017-07-14 19:49 ` Andy Shevchenko 2017-07-14 12:07 ` [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers Arnd Bergmann 2017-07-14 12:52 ` Andy Shevchenko 2017-07-14 19:59 ` Arnd Bergmann 2017-07-14 19:59 ` Arnd Bergmann 2017-07-14 19:59 ` Arnd Bergmann 2017-07-14 12:07 ` [PATCH 19/22] block: DAC960: shut up format-overflow warning Arnd Bergmann 2017-07-14 14:04 ` Jens Axboe 2017-07-14 14:04 ` Jens Axboe 2017-07-14 12:07 ` [PATCH 20/22] sound: pci: avoid string overflow warnings Arnd Bergmann 2017-07-14 12:07 ` Arnd Bergmann 2017-07-14 12:28 ` Takashi Iwai [this message] 2017-07-14 12:28 ` Takashi Iwai 2017-07-18 11:52 ` Arnd Bergmann 2017-07-14 12:07 ` [PATCH 21/22] fscache: fix fscache_objlist_show format processing Arnd Bergmann 2017-09-04 18:29 ` Jérémy Lefaure 2017-07-14 12:07 ` [PATCH 22/22] IB/mlx4: fix sprintf format warning Arnd Bergmann 2017-07-14 13:48 ` Leon Romanovsky 2017-07-25 1:48 ` [PATCH 00/22] gcc-7 -Wformat-* warnings Martin K. Petersen
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=s5hinivfbic.wl-tiwai@suse.de \ --to=tiwai@suse.de \ --cc=akpm@linux-foundation.org \ --cc=alsa-devel@alsa-project.org \ --cc=arnd@arndb.de \ --cc=davem@davemloft.net \ --cc=geliangtang@gmail.com \ --cc=gregkh@linuxfoundation.org \ --cc=jejb@linux.vnet.ibm.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-scsi@vger.kernel.org \ --cc=linux@roeck-us.net \ --cc=martin.petersen@oracle.com \ --cc=netdev@vger.kernel.org \ --cc=o-takashi@sakamocchi.jp \ --cc=perex@perex.cz \ --cc=torvalds@linux-foundation.org \ --cc=x86@kernel.org \ /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: linkBe 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.