* [PATCH] sound/pci/ice1712: replace strcpy() with scnprintf()
@ 2018-03-01 11:45 Joey Pabalinas
2018-03-01 13:55 ` Andy Shevchenko
0 siblings, 1 reply; 7+ messages in thread
From: Joey Pabalinas @ 2018-03-01 11:45 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Jaroslav Kysela, alsa-devel, linux-kernel, Joey Pabalinas
[-- Attachment #1: Type: text/plain, Size: 2605 bytes --]
Replace unsafe uses of strcpy() to copy the name
argument into the sid.name buffer with scnprintf()
to guard against possible buffer overflows.
Even though we don't actually care about the return
value in this specific case, scnprintf() is still
preferred over snprintf() due to scnprintf() returning
the much more logical length of what was *actually* encoded
into the destination array instead of returning length
the resulting string *would* be, assuming it all fit
into the destination array as snprintf() does.
Maybe one day someone will use the return value, and
since the behavior is exactly the same apart from the
return value it would be better to account for that
possibility and program defensively.
Signed-off-by: Joey Pabalinas <joeypabalinas@gmail.com>
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/sound/pci/ice1712/juli.c b/sound/pci/ice1712/juli.c
index 0dbaccf61f33270608..0abacc64168f895d53 100644
--- a/sound/pci/ice1712/juli.c
+++ b/sound/pci/ice1712/juli.c
@@ -26,6 +26,7 @@
#include <linux/delay.h>
#include <linux/interrupt.h>
#include <linux/init.h>
+#include <linux/kernel.h>
#include <linux/slab.h>
#include <sound/core.h>
#include <sound/tlv.h>
@@ -425,10 +426,9 @@ DECLARE_TLV_DB_SCALE(juli_master_db_scale, -6350, 50, 1);
static struct snd_kcontrol *ctl_find(struct snd_card *card,
const char *name)
{
- struct snd_ctl_elem_id sid;
- memset(&sid, 0, sizeof(sid));
- /* FIXME: strcpy is bad. */
- strcpy(sid.name, name);
+ struct snd_ctl_elem_id sid = {0};
+
+ scnprintf(sid.name, sizeof(sid.name), "%s", name);
sid.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
return snd_ctl_find_id(card, &sid);
}
diff --git a/sound/pci/ice1712/quartet.c b/sound/pci/ice1712/quartet.c
index d145b5eb7ff86d978d..332f6226548c6a089a 100644
--- a/sound/pci/ice1712/quartet.c
+++ b/sound/pci/ice1712/quartet.c
@@ -25,6 +25,7 @@
#include <linux/delay.h>
#include <linux/interrupt.h>
#include <linux/init.h>
+#include <linux/kernel.h>
#include <linux/slab.h>
#include <sound/core.h>
#include <sound/tlv.h>
@@ -785,10 +786,9 @@ DECLARE_TLV_DB_SCALE(qtet_master_db_scale, -6350, 50, 1);
static struct snd_kcontrol *ctl_find(struct snd_card *card,
const char *name)
{
- struct snd_ctl_elem_id sid;
- memset(&sid, 0, sizeof(sid));
- /* FIXME: strcpy is bad. */
- strcpy(sid.name, name);
+ struct snd_ctl_elem_id sid = {0};
+
+ scnprintf(sid.name, sizeof(sid.name), "%s", name);
sid.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
return snd_ctl_find_id(card, &sid);
}
--
2.16.2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] sound/pci/ice1712: replace strcpy() with scnprintf()
2018-03-01 11:45 [PATCH] sound/pci/ice1712: replace strcpy() with scnprintf() Joey Pabalinas
@ 2018-03-01 13:55 ` Andy Shevchenko
2018-03-01 14:10 ` Joey Pabalinas
2018-03-01 14:17 ` [PATCH v2] sound/pci/ice1712: replace strcpy() with strlcpy() Joey Pabalinas
0 siblings, 2 replies; 7+ messages in thread
From: Andy Shevchenko @ 2018-03-01 13:55 UTC (permalink / raw)
To: Joey Pabalinas
Cc: Takashi Iwai, Jaroslav Kysela, ALSA Development Mailing List,
Linux Kernel Mailing List
On Thu, Mar 1, 2018 at 1:45 PM, Joey Pabalinas <joeypabalinas@gmail.com> wrote:
> Replace unsafe uses of strcpy() to copy the name
> argument into the sid.name buffer with scnprintf()
> to guard against possible buffer overflows.
> - struct snd_ctl_elem_id sid;
> - memset(&sid, 0, sizeof(sid));
> - /* FIXME: strcpy is bad. */
> - strcpy(sid.name, name);
> + struct snd_ctl_elem_id sid = {0};
> +
> + scnprintf(sid.name, sizeof(sid.name), "%s", name);
So, why not just use strlcpy()?
scnprintf() here adds an overhead for no benefit.
> sid.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
> return snd_ctl_find_id(card, &sid);
> - struct snd_ctl_elem_id sid;
> - memset(&sid, 0, sizeof(sid));
> - /* FIXME: strcpy is bad. */
> - strcpy(sid.name, name);
> + struct snd_ctl_elem_id sid = {0};
> +
> + scnprintf(sid.name, sizeof(sid.name), "%s", name);
> sid.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
> return snd_ctl_find_id(card, &sid);
Ditto.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] sound/pci/ice1712: replace strcpy() with scnprintf()
2018-03-01 13:55 ` Andy Shevchenko
@ 2018-03-01 14:10 ` Joey Pabalinas
2018-03-01 14:17 ` [PATCH v2] sound/pci/ice1712: replace strcpy() with strlcpy() Joey Pabalinas
1 sibling, 0 replies; 7+ messages in thread
From: Joey Pabalinas @ 2018-03-01 14:10 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Takashi Iwai, Jaroslav Kysela, ALSA Development Mailing List,
Linux Kernel Mailing List, Joey Pabalinas
[-- Attachment #1: Type: text/plain, Size: 256 bytes --]
On Thu, Mar 01, 2018 at 03:55:09PM +0200, Andy Shevchenko wrote:
> So, why not just use strlcpy()?
> scnprintf() here adds an overhead for no benefit.
That's a good point, actually. Revising it now; patch
will follow shortly.
--
Joey Pabalinas
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] sound/pci/ice1712: replace strcpy() with strlcpy()
2018-03-01 13:55 ` Andy Shevchenko
2018-03-01 14:10 ` Joey Pabalinas
@ 2018-03-01 14:17 ` Joey Pabalinas
2018-03-01 14:21 ` Andy Shevchenko
2018-03-01 15:13 ` Takashi Iwai
1 sibling, 2 replies; 7+ messages in thread
From: Joey Pabalinas @ 2018-03-01 14:17 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Takashi Iwai, Jaroslav Kysela, ALSA Development Mailing List,
Linux Kernel Mailing List, Joey Pabalinas
[-- Attachment #1: Type: text/plain, Size: 2040 bytes --]
Replace unsafe usages of strcpy() to copy the name
argument into the sid.name buffer with strlcpy()
to guard against possible buffer overflows.
Signed-off-by: Joey Pabalinas <joeypabalinas@gmail.com>
Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/sound/pci/ice1712/juli.c b/sound/pci/ice1712/juli.c
index 0dbaccf61f33270608..21806bab4757241a9e 100644
--- a/sound/pci/ice1712/juli.c
+++ b/sound/pci/ice1712/juli.c
@@ -27,6 +27,7 @@
#include <linux/interrupt.h>
#include <linux/init.h>
#include <linux/slab.h>
+#include <linux/string.h>
#include <sound/core.h>
#include <sound/tlv.h>
@@ -425,10 +426,9 @@ DECLARE_TLV_DB_SCALE(juli_master_db_scale, -6350, 50, 1);
static struct snd_kcontrol *ctl_find(struct snd_card *card,
const char *name)
{
- struct snd_ctl_elem_id sid;
- memset(&sid, 0, sizeof(sid));
- /* FIXME: strcpy is bad. */
- strcpy(sid.name, name);
+ struct snd_ctl_elem_id sid = {0};
+
+ strlcpy(sid.name, name, sizeof(sid.name));
sid.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
return snd_ctl_find_id(card, &sid);
}
diff --git a/sound/pci/ice1712/quartet.c b/sound/pci/ice1712/quartet.c
index d145b5eb7ff86d978d..5bc836241c977feb51 100644
--- a/sound/pci/ice1712/quartet.c
+++ b/sound/pci/ice1712/quartet.c
@@ -26,6 +26,7 @@
#include <linux/interrupt.h>
#include <linux/init.h>
#include <linux/slab.h>
+#include <linux/string.h>
#include <sound/core.h>
#include <sound/tlv.h>
#include <sound/info.h>
@@ -785,10 +786,9 @@ DECLARE_TLV_DB_SCALE(qtet_master_db_scale, -6350, 50, 1);
static struct snd_kcontrol *ctl_find(struct snd_card *card,
const char *name)
{
- struct snd_ctl_elem_id sid;
- memset(&sid, 0, sizeof(sid));
- /* FIXME: strcpy is bad. */
- strcpy(sid.name, name);
+ struct snd_ctl_elem_id sid = {0};
+
+ strlcpy(sid.name, name, sizeof(sid.name));
sid.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
return snd_ctl_find_id(card, &sid);
}
--
2.16.2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] sound/pci/ice1712: replace strcpy() with strlcpy()
2018-03-01 14:17 ` [PATCH v2] sound/pci/ice1712: replace strcpy() with strlcpy() Joey Pabalinas
@ 2018-03-01 14:21 ` Andy Shevchenko
2018-03-01 15:13 ` Takashi Iwai
1 sibling, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2018-03-01 14:21 UTC (permalink / raw)
To: Joey Pabalinas
Cc: Takashi Iwai, Jaroslav Kysela, ALSA Development Mailing List,
Linux Kernel Mailing List
On Thu, Mar 1, 2018 at 4:17 PM, Joey Pabalinas <joeypabalinas@gmail.com> wrote:
> Replace unsafe usages of strcpy() to copy the name
> argument into the sid.name buffer with strlcpy()
> to guard against possible buffer overflows.
>
> Signed-off-by: Joey Pabalinas <joeypabalinas@gmail.com>
> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/sound/pci/ice1712/juli.c b/sound/pci/ice1712/juli.c
> index 0dbaccf61f33270608..21806bab4757241a9e 100644
> --- a/sound/pci/ice1712/juli.c
> +++ b/sound/pci/ice1712/juli.c
> @@ -27,6 +27,7 @@
> #include <linux/interrupt.h>
> #include <linux/init.h>
> #include <linux/slab.h>
> +#include <linux/string.h>
> #include <sound/core.h>
> #include <sound/tlv.h>
>
> @@ -425,10 +426,9 @@ DECLARE_TLV_DB_SCALE(juli_master_db_scale, -6350, 50, 1);
> static struct snd_kcontrol *ctl_find(struct snd_card *card,
> const char *name)
> {
> - struct snd_ctl_elem_id sid;
> - memset(&sid, 0, sizeof(sid));
> - /* FIXME: strcpy is bad. */
> - strcpy(sid.name, name);
> + struct snd_ctl_elem_id sid = {0};
> +
> + strlcpy(sid.name, name, sizeof(sid.name));
> sid.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
> return snd_ctl_find_id(card, &sid);
> }
> diff --git a/sound/pci/ice1712/quartet.c b/sound/pci/ice1712/quartet.c
> index d145b5eb7ff86d978d..5bc836241c977feb51 100644
> --- a/sound/pci/ice1712/quartet.c
> +++ b/sound/pci/ice1712/quartet.c
> @@ -26,6 +26,7 @@
> #include <linux/interrupt.h>
> #include <linux/init.h>
> #include <linux/slab.h>
> +#include <linux/string.h>
> #include <sound/core.h>
> #include <sound/tlv.h>
> #include <sound/info.h>
> @@ -785,10 +786,9 @@ DECLARE_TLV_DB_SCALE(qtet_master_db_scale, -6350, 50, 1);
> static struct snd_kcontrol *ctl_find(struct snd_card *card,
> const char *name)
> {
> - struct snd_ctl_elem_id sid;
> - memset(&sid, 0, sizeof(sid));
> - /* FIXME: strcpy is bad. */
> - strcpy(sid.name, name);
> + struct snd_ctl_elem_id sid = {0};
> +
> + strlcpy(sid.name, name, sizeof(sid.name));
> sid.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
> return snd_ctl_find_id(card, &sid);
> }
> --
> 2.16.2
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] sound/pci/ice1712: replace strcpy() with strlcpy()
2018-03-01 14:17 ` [PATCH v2] sound/pci/ice1712: replace strcpy() with strlcpy() Joey Pabalinas
2018-03-01 14:21 ` Andy Shevchenko
@ 2018-03-01 15:13 ` Takashi Iwai
2018-03-01 15:22 ` Joey Pabalinas
1 sibling, 1 reply; 7+ messages in thread
From: Takashi Iwai @ 2018-03-01 15:13 UTC (permalink / raw)
To: Joey Pabalinas
Cc: Andy Shevchenko, ALSA Development Mailing List, Jaroslav Kysela,
Linux Kernel Mailing List
On Thu, 01 Mar 2018 15:17:07 +0100,
Joey Pabalinas wrote:
>
> Replace unsafe usages of strcpy() to copy the name
> argument into the sid.name buffer with strlcpy()
> to guard against possible buffer overflows.
>
> Signed-off-by: Joey Pabalinas <joeypabalinas@gmail.com>
> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> 2 files changed, 8 insertions(+), 8 deletions(-)
Thanks, applied now.
Takashi
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] sound/pci/ice1712: replace strcpy() with strlcpy()
2018-03-01 15:13 ` Takashi Iwai
@ 2018-03-01 15:22 ` Joey Pabalinas
0 siblings, 0 replies; 7+ messages in thread
From: Joey Pabalinas @ 2018-03-01 15:22 UTC (permalink / raw)
To: Takashi Iwai
Cc: Andy Shevchenko, ALSA Development Mailing List, Jaroslav Kysela,
Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 120 bytes --]
On Thu, Mar 01, 2018 at 04:13:48PM +0100, Takashi Iwai wrote:
> Thanks, applied now.
Cheers
--
Joey Pabalinas
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-03-01 15:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-01 11:45 [PATCH] sound/pci/ice1712: replace strcpy() with scnprintf() Joey Pabalinas
2018-03-01 13:55 ` Andy Shevchenko
2018-03-01 14:10 ` Joey Pabalinas
2018-03-01 14:17 ` [PATCH v2] sound/pci/ice1712: replace strcpy() with strlcpy() Joey Pabalinas
2018-03-01 14:21 ` Andy Shevchenko
2018-03-01 15:13 ` Takashi Iwai
2018-03-01 15:22 ` Joey Pabalinas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).