linux-kernel - Re: [PATCH] scsi:st.c replace snprintf() with sysfs

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: <AA11A711-6652-4A0C-A890-38FAC7D518A2@kolumbus.fi>
Date: Fri, 25 Jul 2025 17:22:57 +0300
From: "Kai Mäkisara (Kolumbus)" <kai.makisara@...umbus.fi>
To: Rujra Bhatt <braker.noob.kernel@...il.com>
Cc: James.Bottomley@...senpartnership.com,
 martin.petersen@...cle.com,
 linux-scsi@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 Shuah Khan <skhan@...uxfoundation.org>,
 linux-kernel-mentees@...ts.linux.dev,
 bvanassche@....org
Subject: Re: [PATCH] scsi:st.c replace snprintf() with sysfs_emit()


> On 24. Jul 2025, at 3.58, Rujra Bhatt <braker.noob.kernel@...il.com> wrote:
> 
> replace snprintf() with sysfs_emit() or sysfs_emit_at() in st.c file to
> follow kernel's guidelines from Documentation/filesystems/sysfs.rst
> This improves safety, consistency and easier to maintain and update it
> in the future.
> 
Bart already wrote that these changes are required for existing drivers. I am a little hesitant about these kind of changes that don't fix anything  and don't add improvements. Any changes risk breaking something.

But, since I have wondered what these sysfs_emit* functions do, I decided to take a look...

> Signed-off-by: Rujra Bhatt <braker.noob.kernel@...il.com>
> ---
> drivers/scsi/st.c | 42 +++++++++++++++++++++---------------------
> 1 file changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> index 74a6830b7ed8..38badba472d7 100644
> --- a/drivers/scsi/st.c
> +++ b/drivers/scsi/st.c
> @@ -4564,25 +4564,25 @@ module_exit(exit_st);
> /* The sysfs driver interface. Read-only at the moment */
> static ssize_t try_direct_io_show(struct device_driver *ddp, char *buf)
> {
> -       return scnprintf(buf, PAGE_SIZE, "%d\n", try_direct_io);
> +       return sysfs_emit_at(buf, PAGE_SIZE, "%d\n", try_direct_io);

This "maps back" to vscnprintf(buf + PAGE_SIZE, 0, "%d\n", try_direct_io), which does not match the existing code (i.e., would return nothing). (Actually the problem is caught already by the check at the beginning of sysfs_emit_at()).

I am not happy to find out that this patch seems not to have been tested before submission. (Well, maybe compile tested.)

Kai