[PATCH] ALSA: mixer_oss: Replace deprecated strcpy() with strscpy()

[PATCH] ALSA: mixer_oss: Replace deprecated strcpy() with strscpy()

[Thorsten Blum]

strcpy() is deprecated; use strscpy() instead.

No functional changes intended.

Link: https://github.com/KSPP/linux/issues/88
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 sound/core/oss/mixer_oss.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/core/oss/mixer_oss.c b/sound/core/oss/mixer_oss.c
index 05fc8911479c..d3d993da3269 100644
--- a/sound/core/oss/mixer_oss.c
+++ b/sound/core/oss/mixer_oss.c
@@ -1014,11 +1014,11 @@ static int snd_mixer_oss_build_input(struct snd_mixer_oss *mixer,
 			
 		if (kctl->info(kctl, uinfo))
 			return 0;
-		strcpy(str, ptr->name);
+		strscpy(str, ptr->name);
 		if (!strcmp(str, "Master"))
-			strcpy(str, "Mix");
+			strscpy(str, "Mix");
 		if (!strcmp(str, "Master Mono"))
-			strcpy(str, "Mix Mono");
+			strscpy(str, "Mix Mono");
 		slot.capture_item = 0;
 		if (!strcmp(uinfo->value.enumerated.name, str)) {
 			slot.present |= SNDRV_MIXER_OSS_PRESENT_CAPTURE;
-- 
2.49.0

Re: [PATCH] ALSA: mixer_oss: Replace deprecated strcpy() with strscpy()

[Al Viro]

On Thu, Jun 19, 2025 at 12:36:29AM +0200, Thorsten Blum wrote:
> strcpy() is deprecated; use strscpy() instead.
> 
> No functional changes intended.

Have you actually read the damn thing?  Seriously, look at the uses
of 'str' downstream.  The only thing it is ever passed to is strcmp().

In other words, why do we need to copy it anywhere?  What's wrong with
having char *str instead of that array and replacing strcpy() with
plain and simple pointer assignment?

Re: [PATCH] ALSA: mixer_oss: Replace deprecated strcpy() with strscpy()

[Thorsten Blum]

On 19. Jun 2025, at 00:49, Al Viro wrote:
> On Thu, Jun 19, 2025 at 12:36:29AM +0200, Thorsten Blum wrote:
>> strcpy() is deprecated; use strscpy() instead.
>> 
>> No functional changes intended.
> 
> Have you actually read the damn thing?  Seriously, look at the uses
> of 'str' downstream.  The only thing it is ever passed to is strcmp().
> 
> In other words, why do we need to copy it anywhere?  What's wrong with
> having char *str instead of that array and replacing strcpy() with
> plain and simple pointer assignment?

I read it, but didn't question whether copying was actually necessary.

However, it looks like 'ptr->name' can originate from userland (via proc
file - see the function comment), which could make using 'char *str'
directly unsafe, unless I'm missing something.

Something like this would skip one copy while keeping it safe:

char tmp_str[64];
char *str;

strscpy(tmp_str, ptr->name);
if (!strcmp(tmp_str, "Master"))
	str = "Mix";
else if (!strcmp(tmp_str, "Master Mono"))
	str = "Mix Mono";
else
	str = tmp_str;

Thanks,
Thorsten

Re: [PATCH] ALSA: mixer_oss: Replace deprecated strcpy() with strscpy()

[Takashi Iwai]

On Thu, 19 Jun 2025 14:50:04 +0200,
Thorsten Blum wrote:
> 
> On 19. Jun 2025, at 00:49, Al Viro wrote:
> > On Thu, Jun 19, 2025 at 12:36:29AM +0200, Thorsten Blum wrote:
> >> strcpy() is deprecated; use strscpy() instead.
> >> 
> >> No functional changes intended.
> > 
> > Have you actually read the damn thing?  Seriously, look at the uses
> > of 'str' downstream.  The only thing it is ever passed to is strcmp().
> > 
> > In other words, why do we need to copy it anywhere?  What's wrong with
> > having char *str instead of that array and replacing strcpy() with
> > plain and simple pointer assignment?
> 
> I read it, but didn't question whether copying was actually necessary.
> 
> However, it looks like 'ptr->name' can originate from userland (via proc
> file - see the function comment), which could make using 'char *str'
> directly unsafe, unless I'm missing something.
> 
> Something like this would skip one copy while keeping it safe:
> 
> char tmp_str[64];
> char *str;
> 
> strscpy(tmp_str, ptr->name);
> if (!strcmp(tmp_str, "Master"))
> 	str = "Mix";
> else if (!strcmp(tmp_str, "Master Mono"))
> 	str = "Mix Mono";
> else
> 	str = tmp_str;

Al is right, we should optimize it instead.  As it's been already a
string copied to a kernel, and the string is certainly NUL-terminated,
hence there is no need to worry about using the pointer.
It'd be something like:

--- a/sound/core/oss/mixer_oss.c
+++ b/sound/core/oss/mixer_oss.c
@@ -991,7 +991,7 @@ static int snd_mixer_oss_build_input(struct snd_mixer_oss *mixer,
 	struct slot *pslot;
 	struct snd_kcontrol *kctl;
 	struct snd_mixer_oss_slot *rslot;
-	char str[64];	
+	const char *str;
 	
 	/* check if already assigned */
 	if (mixer->slots[ptr->oss_id].get_volume && ! replace_old)
@@ -1014,11 +1014,11 @@ static int snd_mixer_oss_build_input(struct snd_mixer_oss *mixer,
 			
 		if (kctl->info(kctl, uinfo))
 			return 0;
-		strcpy(str, ptr->name);
+		str = ptr->name;
 		if (!strcmp(str, "Master"))
-			strcpy(str, "Mix");
-		if (!strcmp(str, "Master Mono"))
-			strcpy(str, "Mix Mono");
+			str = "Mix";
+		else if (!strcmp(str, "Master Mono"))
+			str = "Mix Mono";
 		slot.capture_item = 0;
 		if (!strcmp(uinfo->value.enumerated.name, str)) {
 			slot.present |= SNDRV_MIXER_OSS_PRESENT_CAPTURE;


thanks,

Takashi