On 2/10/2024 8:20 AM, Fenghua Yu wrote:
Hi, Lijun,
On 2/9/24 13:53, Lijun Pan wrote:
On 2/10/2024 3:14 AM, Fenghua Yu wrote:
If CONFIG_HARDENED_USERCOPY is enabled, copying completion record from
event log cache to user triggers a kernel bug.
...
Fixes: c2f156bf168f ("dmaengine: idxd: create kmem cache for event
log fault items")
Reported-by: Tony Zhu <tony.zhu@xxxxxxxxx>
Tested-by: Tony Zhu <tony.zhu@xxxxxxxxx>
Signed-off-by: Fenghua Yu <fenghua.yu@xxxxxxxxx>
Reviewed-by: Lijun Pan <lijun.pan@xxxxxxxxx>
---
drivers/dma/idxd/init.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 14df1f1347a8..4954adc6bb60 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -343,7 +343,9 @@ static void idxd_cleanup_internals(struct
idxd_device *idxd)
static int idxd_init_evl(struct idxd_device *idxd)
{
struct device *dev = &idxd->pdev->dev;
+ unsigned int evl_cache_size;
struct idxd_evl *evl;
+ const char *idxd_name;
if (idxd->hw.gen_cap.evl_support == 0)
return 0;
@@ -355,9 +357,16 @@ static int idxd_init_evl(struct idxd_device *idxd)
spin_lock_init(&evl->lock);
evl->size = IDXD_EVL_SIZE_MIN;
- idxd->evl_cache = kmem_cache_create(dev_name(idxd_confdev(idxd)),
- sizeof(struct idxd_evl_fault) +
evl_ent_size(idxd),
- 0, 0, NULL);
+ idxd_name = dev_name(idxd_confdev(idxd));
+ evl_cache_size = sizeof(struct idxd_evl_fault) +
evl_ent_size(idxd);
+ /*
+ * Since completion record in evl_cache will be copied to user
+ * when handling completion record page fault, need to create
+ * the cache suitable for user copy.
+ */
Maybe briefly compare kmem_cache_create() with
kmem_cache_create_usercopy() and add up to the above comments. If you
think it too verbose, then forget about it.
It's improper to add comment to compare the two functions here because:
1. When people look into this code in init.c, they have no idea why
compare the functions here when only kmem_cache_create_usercopy() is
used. The comparison is only meaningful in this patch's context and has
been explained in the patch commit message.
2. Comparison or any details of the function can be found easily in the
function implementation. No need to add more details on top of the
current comment which covers enough info (i.e. why call this function)
already.
Given the above reasons, I will keep the current comment and patch
without change.
That's fine.
Lijun
