Re: [PATCH] dma-direct: use the correct size for dma_set_encrypted()

2022-06-23 Thread Christoph Hellwig 
On Thu, Jun 23, 2022 at 07:00:58AM +, Dexuan Cui wrote:
> It looks like commit 4a37f3dd9a831 fixed a different issue?
> 
> Here my patch is for the latest mainline:
> 
> In dma_direct_alloc()'s error handling path, we pass 'size' to 
> dma_set_encrypted():
>   out_encrypt_pages:
> if (dma_set_encrypted(dev, page_address(page), size))
> 
> However, in dma_direct_free(), we pass ' 1 << page_order ' to 
> dma_set_encrypted().
> I think the ' 1 << page_order' is incorrect and it should be 'size' as well?

Indeed.  I've applied the patch now.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] dma-direct: use the correct size for dma_set_encrypted()

2022-06-23 Thread Robin Murphy 

On 2022-06-23 08:00, Dexuan Cui wrote:

From: Christoph Hellwig 
Sent: Wednesday, June 22, 2022 10:44 PM
To: Dexuan Cui 
  ...
On Wed, Jun 22, 2022 at 12:14:24PM -0700, Dexuan Cui wrote:

The third parameter of dma_set_encrypted() is a size in bytes rather than
the number of pages.

Fixes: 4d0564785bb0 ("dma-direct: factor out dma_set_{de,en}crypted

helpers")

Signed-off-by: Dexuan Cui 


see:

commit 4a37f3dd9a83186cb88d44808ab35b78375082c9 (tag:
dma-mapping-5.19-2022-05-25)
Author: Robin Murphy 
Date:   Fri May 20 18:10:13 2022 +0100

 dma-direct: don't over-decrypt memory


It looks like commit 4a37f3dd9a831 fixed a different issue?

Here my patch is for the latest mainline:

In dma_direct_alloc()'s error handling path, we pass 'size' to 
dma_set_encrypted():
   out_encrypt_pages:
  if (dma_set_encrypted(dev, page_address(page), size))

However, in dma_direct_free(), we pass ' 1 << page_order ' to 
dma_set_encrypted().
I think the ' 1 << page_order' is incorrect and it should be 'size' as well?


I think technically you're both right - these instances clearly have a 
history tracing back to the original bug that my patch addressed, but 
the refactoring then made them into their own distinct bug in terms of 
the internal dma_set_encrypted() interface, per the commit message here. 
Apparently I failed to spot this when forward-porting 4a37f3dd9a831 from 
5.10 (as the commit message says, don't ask... ;) ) - I guess I was only 
looking at where the set_memory_*() callsites had moved to. For this patch,


Reviewed-by: Robin Murphy 

Thanks
Robin.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: [PATCH] dma-direct: use the correct size for dma_set_encrypted()

2022-06-23 Thread Dexuan Cui via iommu 
> From: Christoph Hellwig 
> Sent: Wednesday, June 22, 2022 10:44 PM
> To: Dexuan Cui 
>  ...
> On Wed, Jun 22, 2022 at 12:14:24PM -0700, Dexuan Cui wrote:
> > The third parameter of dma_set_encrypted() is a size in bytes rather than
> > the number of pages.
> >
> > Fixes: 4d0564785bb0 ("dma-direct: factor out dma_set_{de,en}crypted
> helpers")
> > Signed-off-by: Dexuan Cui 
> 
> see:
> 
> commit 4a37f3dd9a83186cb88d44808ab35b78375082c9 (tag:
> dma-mapping-5.19-2022-05-25)
> Author: Robin Murphy 
> Date:   Fri May 20 18:10:13 2022 +0100
> 
> dma-direct: don't over-decrypt memory

It looks like commit 4a37f3dd9a831 fixed a different issue?

Here my patch is for the latest mainline:

In dma_direct_alloc()'s error handling path, we pass 'size' to 
dma_set_encrypted():
  out_encrypt_pages:
  if (dma_set_encrypted(dev, page_address(page), size))

However, in dma_direct_free(), we pass ' 1 << page_order ' to 
dma_set_encrypted().
I think the ' 1 << page_order' is incorrect and it should be 'size' as well?

Thanks,
-- Dexuan

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] dma-direct: use the correct size for dma_set_encrypted()

2022-06-22 Thread Christoph Hellwig 
On Wed, Jun 22, 2022 at 12:14:24PM -0700, Dexuan Cui wrote:
> The third parameter of dma_set_encrypted() is a size in bytes rather than
> the number of pages.
> 
> Fixes: 4d0564785bb0 ("dma-direct: factor out dma_set_{de,en}crypted helpers")
> Signed-off-by: Dexuan Cui 

see:

commit 4a37f3dd9a83186cb88d44808ab35b78375082c9 (tag: 
dma-mapping-5.19-2022-05-25)
Author: Robin Murphy 
Date:   Fri May 20 18:10:13 2022 +0100

dma-direct: don't over-decrypt memory
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH] dma-direct: use the correct size for dma_set_encrypted()

2022-06-22 Thread Dexuan Cui via iommu 
The third parameter of dma_set_encrypted() is a size in bytes rather than
the number of pages.

Fixes: 4d0564785bb0 ("dma-direct: factor out dma_set_{de,en}crypted helpers")
Signed-off-by: Dexuan Cui 
---
 kernel/dma/direct.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index e978f36e6be8..8d0b68a17042 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -357,7 +357,7 @@ void dma_direct_free(struct device *dev, size_t size,
} else {
if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_CLEAR_UNCACHED))
arch_dma_clear_uncached(cpu_addr, size);
-   if (dma_set_encrypted(dev, cpu_addr, 1 << page_order))
+   if (dma_set_encrypted(dev, cpu_addr, size))
return;
}
 
@@ -392,7 +392,6 @@ void dma_direct_free_pages(struct device *dev, size_t size,
struct page *page, dma_addr_t dma_addr,
enum dma_data_direction dir)
 {
-   unsigned int page_order = get_order(size);
void *vaddr = page_address(page);
 
/* If cpu_addr is not from an atomic pool, dma_free_from_pool() fails */
@@ -400,7 +399,7 @@ void dma_direct_free_pages(struct device *dev, size_t size,
dma_free_from_pool(dev, vaddr, size))
return;
 
-   if (dma_set_encrypted(dev, vaddr, 1 << page_order))
+   if (dma_set_encrypted(dev, vaddr, size))
return;
__dma_direct_free_pages(dev, page, size);
 }
-- 
2.25.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu