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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <DA7484EA-83F7-496A-AB9F-2370BBBC0883@linux.dev>
Date: Thu, 19 Jun 2025 14:50:04 +0200
From: Thorsten Blum <thorsten.blum@...ux.dev>
To: Al Viro <viro@...iv.linux.org.uk>
Cc: Jaroslav Kysela <perex@...ex.cz>,
 Takashi Iwai <tiwai@...e.com>,
 Christophe JAILLET <christophe.jaillet@...adoo.fr>,
 Takashi Iwai <tiwai@...e.de>,
 linux-sound@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ALSA: mixer_oss: Replace deprecated strcpy() with
 strscpy()

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