linux-kernel - Re: [PATCH] crypto: ccp/sfs - Use DIV_ROUND_UP for set_memory

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: <20251001144431.247305-1-Ashish.Kalra@amd.com>
Date: Wed, 1 Oct 2025 14:44:31 +0000
From: Ashish Kalra <Ashish.Kalra@....com>
To: <kfatyuip@...il.com>
CC: <davem@...emloft.net>, <herbert@...dor.apana.org.au>,
	<john.allen@....com>, <linux-crypto@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <thomas.lendacky@....com>
Subject: Re: [PATCH] crypto: ccp/sfs - Use DIV_ROUND_UP for set_memory_uc() size calculation

>From: Kieran Moy <kfatyuip@...il.com>

>The SFS driver allocates a 2MB command buffer using snp_alloc_hv_fixed_pages(),
>but set_memory_uc() is called with SFS_NUM_PAGES_CMDBUF which assumes 4KB pages.
>This mismatch could lead to incomplete cache attribute updates on the buffer if
>the payload size is not strictly page-aligned.

>Switch to using DIV_ROUND_UP(SFS_MAX_PAYLOAD_SIZE, PAGE_SIZE) to calculate the
>number of pages required for the attribute update. This approach follows kernel
>coding best practices, improves code robustness, and ensures that all buffer
>regions are properly covered regardless of current or future PAGE_SIZE values.

>Using DIV_ROUND_UP is also consistent with Linux kernel style for page counting,
>which avoids hidden bugs in case the payload size ever changes and is not a
>multiple of PAGE_SIZE, or if the kernel is built with a non-default page size.

>Signed-off-by: Kieran Moy <kfatyuip@...il.com>
>---
> drivers/crypto/ccp/sfs.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)

>diff --git a/drivers/crypto/ccp/sfs.c b/drivers/crypto/ccp/sfs.c
>index 2f4beaafe7ec..3397895160c0 100644
>--- a/drivers/crypto/ccp/sfs.c
>+++ b/drivers/crypto/ccp/sfs.c
>@@ -277,7 +277,8 @@ int sfs_dev_init(struct psp_device *psp)
> 	/*
> 	* SFS command buffer must be mapped as non-cacheable.
> 	*/
>-	ret = set_memory_uc((unsigned long)sfs_dev->command_buf, SFS_NUM_PAGES_CMDBUF);
>+	ret = set_memory_uc((unsigned long)sfs_dev->command_buf,
>+			   DIV_ROUND_UP(SFS_MAX_PAYLOAD_SIZE, PAGE_SIZE));

I am not sure, i find the maths quite straightforward: 

The command buffer is allocated using snp_alloc_hv_fixed_pages() or alloc_pages() using
an order of 9, which should be allocating 512 pages and returning a 2MB aligned address 
and then set_memory_uc() is (SFS_MAX_PAYLOAD_SIZE / PAGE_SIZE) which computes 512 pages.
Also the payload size here is page-aligned and is a constant.

Thanks,
Ashish

> 	if (ret) {
> 		dev_dbg(dev, "Set memory uc failed\n");
> 		goto cleanup_cmd_buf;