Re: [PATCH 06/10] dma: Use free_decrypted_pages()

From: Edgecombe, Rick P
Date: Mon Oct 23 2023 - 13:28:04 EST


On Mon, 2023-10-23 at 18:22 +0100, Robin Murphy wrote:
> > >
> > > If something's needed in the fallback path here, what about the
> > > cma_release() paths?
> >
> > You mean inside cma_release(). If so, unfortunately I think it
> > won't
> > fit great because there are callers that are never dealing with
> > shared
> > memory (huge tlb). The reset-to-private operation does extra work
> > that
> > would be nice to avoid when possible.
> >
> > The cases I thought exhibited the issue were the two calls sites of
> > dma_set_decrypted(). Playing around with it, I was thinking it
> > might be
> > easier to just fix those to open code leaking the pages on
> > dma_set_decrypted() error. In which case it won't have the re-
> > encrypt
> > problem.
> >
> > It make's it less fool proof, but more efficient. And
> > free_decrypted_pages() doesn't fit great anyway, as pointed out by
> > Christoph.
>
> My point is that in dma_direct_alloc(), we get some memory either
> straight from the page allocator *or* from a CMA area, then call
> set_memory_decrypted() on it. If the problem is that
> set_memory_decrypted() can fail and require cleanup, then logically
> if
> that cleanup is necessary for the dma_free_contiguous()-
> >__free_pages()
> call, then surely it must also be necessary for the
> dma_free_contiguous()->cma_release()->free_contig_range()-
> >__free_page()
> calls.

Oh, I see you are saying the patch misses that case. Yes, makes sense.

Sorry for the confusion. In trying to fix the callers, I waded through
a lot of area's that I didn't have much expertise in and probably
should have marked the whole thing RFC.