On 05-06-2023 09:42, Kevin Brodsky wrote:
On 04/04/2023 15:53, Tudor Cretu wrote:
The aio_ring buffer is a shared memory region that hosts the io_event array. The io_event struct may contain user pointers, so the memory region must be allowed to store and load capability pointers.
Signed-off-by: Tudor Cretu tudor.cretu@arm.com
fs/aio.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/fs/aio.c b/fs/aio.c index e6e45498ccd27..a83bf7f656ca4 100644 --- a/fs/aio.c +++ b/fs/aio.c @@ -466,6 +466,16 @@ static int aio_ring_mmap(struct file *file, struct vm_area_struct *vma) { vma->vm_flags |= VM_DONTEXPAND; vma->vm_ops = &aio_ring_vm_ops;
- if (IS_ENABLED(CONFIG_CHERI_PURECAP_UABI)) {
size_t sz = vma->vm_end - vma->vm_start;struct kioctx *ctx = file->f_mapping->private_data;unsigned long pfn = page_to_pfn(ctx->ring_pages[0]);This looks suspicious: we allocated the pages individually so there is no guarantee that they are physically contiguous. Maybe we don't need to do that at all though, see below.
vma->vm_flags |= VM_READ_CAPS | VM_WRITE_CAPS;vma_set_page_prot(vma);return remap_pfn_range(vma, vma->vm_start, pfn, sz, vma->vm_page_prot);It is not clear to me why we need to manually map these pages ahead of time to get capability access. I can see that this is how it is done in io_uring, but I think the situation is quite different there: there is no actual filesystem involved, so the .mmap handler has to do all the work itself. OTOH in AIO we have a pseudo-filesystem and actually create a file for the ring, allowing us to use the standard filemap_fault callback in our vm_operations_struct. Wouldn't it be sufficient to add our vm_flags here to get them used by filemap_fault?
Indeed, well spotted! I think I encountered some issue and decided to do this, but just forgot to check that it really needs remapping, which indeed, it doesn't need them. Thanks, this definitely makes it neater.
Tudor
Kevin