On Tue, 31 Oct 2023 15:54:52 +0000 "Edgecombe, Rick P" <rick.p.edgecombe@xxxxxxxxx> wrote: > On Tue, 2023-10-31 at 11:43 +0100, Petr Tesařík wrote: > > > > I admit I'm not familiar with the encryption/decryption API, but if a > > __free_pages() is not sufficient here, then it is quite confusing. > > The error label is reached only if set_memory_decrypted() returns > > non-zero. My naive expectation is that the memory is *not* decrypted > > in > > that case and does not require special treatment. Is this assumption > > wrong? > > Yea, the memory can still be decrypted, or partially decrypted. On x86, > all the set_memory() calls can fail part way through the work, and they > don't rollback the changes they had made up to that point. Thank you for the explanation. So, after set_memory_decrypted() fails, the pages become Schroedinger-crypted, but since its true state cannot be observed by the guest kernel, it stays as such forever. Sweet. >[...] > > OTOH I believe there is a bug in the logic. The subsequent > > __free_pages() in swiotlb_alloc_tlb() would have to be changed to a > > free_decrypted_pages(). However, I'm proposing a different approach > > to > > address the latter issue here: > > > > https://lore.kernel.org/linux-iommu/20231026095123.222-1-petrtesarik@xxxxxxxxxxxxxxx/T/ > > Oh, yes, that makes sense. I was planning to send a patch to just leak > the pages if set_memory_decrypted() fails, after my v2 linked above is > accepted. It could have a different label than the phys_limit check > error path added in your linked patch, so that case would still free > the perfectly fine encrypted pages. Hm, should I incorporate this knowledge into a v2 of my patch and address both issues? Petr T
