linux-kernel - RE: We found a bug in skx_common.c for the latest linux

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <SJ1PR11MB608322D580CC951F58A03B1DFC4BA@SJ1PR11MB6083.namprd11.prod.outlook.com>
Date: Fri, 11 Jul 2025 15:24:34 +0000
From: "Luck, Tony" <tony.luck@...el.com>
To: "Zhuo, Qiuxu" <qiuxu.zhuo@...el.com>, Wang Haoran
	<haoranwangsec@...il.com>
CC: "bp@...en8.de" <bp@...en8.de>, "linux-edac@...r.kernel.org"
	<linux-edac@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: We found a bug in skx_common.c for the latest linux

> Although there isn't a real *issue" you described here in the {skx, i10nm}_edac driver,
> it seems still worthwhile to replace snprintf() with scnprintf() to enhance code robustness.
>
>     - snprintf() returns the would-be-output size.
>
>     - scnprintf() returns the actual number of characters written to the buffer.
>                           This ensures the subsequent calls do not exceed the buffer limit,
>                           particularly in code like: len += scnprintf(buf+len, SIZE - len, ...)
>
> It's OK to me that Haoran replaces snprintf() with scnprintf() and applies these changes to
> the files: skx_common.c, skx_base.c,  and i10nm_base.c as well.
>
> @Luck, Tony, do you agree with Haoran replacing snprintf() with scnprintf()
> even though no issues are currently occurring in the {skx,i10nm}_edac drivers?

Agreed. It would be a pain to debug if the first snprintf() to the buffer did overflow.
So code robustness is important.

Haoran: Please send a properly formatted patch with your "Signed-off-by:" line
included.  You've provided plenty of explanation in this e-mail thread (which
I'll "Link:" when I commit. So it's OK to be quite terse in the commit message.

Something like:

  snprintf() is fragile when its return value will be used to append additional data
  to a buffer. Use scnprintf() instead.

-Tony