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: <CY8PR11MB713497AF56976DEFD1B8EB3E894BA@CY8PR11MB7134.namprd11.prod.outlook.com>
Date: Fri, 11 Jul 2025 06:16:05 +0000
From: "Zhuo, Qiuxu" <qiuxu.zhuo@...el.com>
To: Wang Haoran <haoranwangsec@...il.com>, "Luck, Tony" <tony.luck@...el.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

Hi Haoran,

> From: Wang Haoran <haoranwangsec@...il.com>
> [...]
> Subject: Re: We found a bug in skx_common.c for the latest linux
> 
> Yes, I have!
> Thank you for your reply.
> This issue was discovered through static analysis rather than at runtime, so
> unfortunately we do not have a real-world PoC or specific dmesg log from
> skx_show_retry_rd_err_log().
> To provide additional context, during the execution of
> skx_show_retry_rd_err_log(res, skx_msg + len, MSG_SIZE - len, scrub_err); if
> len is greater than the allocated MSG_SIZE, the subsequent call inside
> skx_show_retry_rd_err_log such as n = snprintf(msg, len, "
> retry_rd_err_log[%.8x %.8x %.8x %.8x %.8x]", log0, log1, log2, log3, log4); will
> pass an invalid address (skx_msg + len) that has exceeded the bounds of the
> skx_msg buffer. Moreover, the size parameter (len) becomes negative, which

The size of skx_msg is 1024. The max "len" is ~420 before entering
skx_show_retry_rd_err_log(), which is significantly less than 1024. 
Therefore, it's safe to call skx_show_retry_rd_err_log().

> is treated as a very large positive number due to the use of size_t. This can
> result in out-of-bounds memory access and potentially dangerous behavior in
> the kernel.

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?

Thanks!
-Qiuxu