From: HariKrishna Sagala hariconscious@gmail.com
snprintf() returns the would-be-filled size when the string overflows the given buffer size, hence using this value may result in a buffer overflow (although it's unrealistic).
This patch replaces it with a safer version, scnprintf() for papering over such a potential issue. Link: https://github.com/KSPP/linux/issues/105 'Fixes: 5a565ba23abe ("ASoC: Intel: avs: Probing and firmware tracing over debugfs")'
Signed-off-by: HariKrishna Sagala hariconscious@gmail.com --- Thank you for the feedback and the suggestions. Corrected the indentation & commit message. V1: https://lore.kernel.org/all/20251112120235.54328-2-hariconscious@gmail.com/ sound/soc/intel/avs/debugfs.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/intel/avs/debugfs.c b/sound/soc/intel/avs/debugfs.c index 3534de46f9e4..cdb82392b9ee 100644 --- a/sound/soc/intel/avs/debugfs.c +++ b/sound/soc/intel/avs/debugfs.c @@ -119,9 +119,9 @@ static ssize_t probe_points_read(struct file *file, char __user *to, size_t coun }
for (i = 0; i < num_desc; i++) { - ret = snprintf(buf + len, PAGE_SIZE - len, - "Id: %#010x Purpose: %d Node id: %#x\n", - desc[i].id.value, desc[i].purpose, desc[i].node_id.val); + ret = scnprintf(buf + len, PAGE_SIZE - len, + "Id: %#010x Purpose: %d Node id: %#x\n", + desc[i].id.value, desc[i].purpose, desc[i].node_id.val); if (ret < 0) goto free_desc; len += ret;
base-commit: 24172e0d79900908cf5ebf366600616d29c9b417
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree. Subject: [PATCH v2] ASoC: Intel: avs: Fix potential buffer overflow by snprintf() Link: https://lore.kernel.org/stable/20251112181851.13450-1-hariconscious%40gmail....
On Wed, Nov 12, 2025 at 11:48:51PM +0530, hariconscious@gmail.com wrote:
From: HariKrishna Sagala hariconscious@gmail.com
snprintf() returns the would-be-filled size when the string overflows the given buffer size, hence using this value may result in a buffer overflow (although it's unrealistic).
unrealistic == impossible
So why make this change at all?
This patch replaces it with a safer version, scnprintf() for papering over such a potential issue.
Don't "paper over", actually fix real things.
Link: https://github.com/KSPP/linux/issues/105 'Fixes: 5a565ba23abe ("ASoC: Intel: avs: Probing and firmware tracing over debugfs")'
No, this is not a "fix".
Also please do not wrap lines of fixes tags.
thanks,
greg k-h
On Wed, Nov 12, 2025 at 07:33:51PM +0000, Mark Brown wrote:
On Wed, Nov 12, 2025 at 02:20:19PM -0500, Greg KH wrote:
Also please do not wrap lines of fixes tags.
Someone probably ought to teach checkpatch about that one, it moans about long lines without checking for Fixes: IIRC.
I can recall this issue, too... I checked how to reproduce and fix this but it seems it's already done: I couldn't reproduce it. I'm getting this for breaking a Fixes: line:
WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: ...
It basically now checks the subject matches with the quoted string.
So all is well!
On 2025-11-12 8:20 PM, Greg KH wrote:
On Wed, Nov 12, 2025 at 11:48:51PM +0530, hariconscious@gmail.com wrote:
From: HariKrishna Sagala hariconscious@gmail.com
snprintf() returns the would-be-filled size when the string overflows the given buffer size, hence using this value may result in a buffer overflow (although it's unrealistic).
unrealistic == impossible
So why make this change at all?
The problem will never occur in production-scenario given the AudioDSP firmware limitation - max ~10 probe-point entries so, the built string will be far away from 4K_SZ bytes.
If the verdict is: ignore the recommendation as the problem is unrealistic, I'm OK with that. Typically though I'd prefer to stick to the recommendations.
This patch replaces it with a safer version, scnprintf() for papering over such a potential issue.
Don't "paper over", actually fix real things.
Link: https://github.com/KSPP/linux/issues/105 'Fixes: 5a565ba23abe ("ASoC: Intel: avs: Probing and firmware tracing over debugfs")'
No, this is not a "fix".
The patch isn't worded well, that's clear. While the patch is an outcome of static-analysis, isn't it good to have 'Fixes:' to point out the offending commit regardless?
On Thu, Nov 13, 2025 at 09:46:12AM +0100, Cezary Rojewski wrote:
On 2025-11-12 8:20 PM, Greg KH wrote:
On Wed, Nov 12, 2025 at 11:48:51PM +0530, hariconscious@gmail.com wrote:
From: HariKrishna Sagala hariconscious@gmail.com
snprintf() returns the would-be-filled size when the string overflows the given buffer size, hence using this value may result in a buffer overflow (although it's unrealistic).
unrealistic == impossible
So why make this change at all?
The problem will never occur in production-scenario given the AudioDSP firmware limitation - max ~10 probe-point entries so, the built string will be far away from 4K_SZ bytes.
If the verdict is: ignore the recommendation as the problem is unrealistic, I'm OK with that. Typically though I'd prefer to stick to the recommendations.
That's fine, but don't claim that it fixes a buffer overflow when that is NOT what this is doing at all.
This patch replaces it with a safer version, scnprintf() for papering over such a potential issue.
Don't "paper over", actually fix real things.
Link: https://github.com/KSPP/linux/issues/105 'Fixes: 5a565ba23abe ("ASoC: Intel: avs: Probing and firmware tracing over debugfs")'
No, this is not a "fix".
The patch isn't worded well, that's clear. While the patch is an outcome of static-analysis, isn't it good to have 'Fixes:' to point out the offending commit regardless?
No, it is not "fixing" anything. Please don't claim that it does. It is "just" a code transformation to get rid of an api that some people do not like.
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org