On 22/01/2024 10:00, Amit Daniel Kachhap wrote:
+bool reserv_vmi_valid_capability(struct vma_iterator *vmi, user_uintptr_t cap, bool locked) +{ + struct vm_area_struct *vma; + struct mm_struct *mm = current->mm; + ptraddr_t cap_start = cheri_base_get(cap); + ptraddr_t cap_end = cap_start + cheri_length_get(cap); + bool ret = false;
+ if (!test_bit(MMF_PCUABI_RESERVE, &mm->flags)) + return true;
+ if (!locked && mmap_read_lock_killable(mm)) + return false;
+ /* Check if there is match with the existing reservations */ + vma = mas_find(&vmi->mas, cap_end); + if (!vma) { + ret = true;
Why do we return true if there is no reservation in this range?
Well this is somewhat inspired from munmap() behaviour. If munmap() is called for non-existing vma then it does not return error. So I tried to have the same behavior and not return the -ERESERVATION error.
Ah, I see what you're saying. The relevant rule in the spec is:
Its bounds are checked against existing reservations. If the bounds of the capability are not contained within the bounds of any existing reservation, the call fails with -ERESERVATION.
The reason there is no exception for munmap() is that passing an owning capability corresponding to a reservation that no longer exists is a form of use-after-free: such capabilities should not be used in any way after the reservation is gone. With an appropriate revocation scheme, they would be untagged before this can happen, so the syscall would fail anyway. Having this explicit check catches at least some of the cases that revocation would.
Besides I need some clarification regarding this interface. Currently this interface only checks reservation for the first vma and not all vma in the given range. As reservation fields remains constant so I left it like this for performance benefit. Let me know your opinion on this.
I don't think this is needed, because reservations cannot overlap. If the capability bounds are within the reservation according to the first vma we find, then we're good. If we do find a vma but the capability bounds exceed the reservation bounds, then it means that this capability owns a (larger) reservation that has been destroyed and replaced later by a smaller one, so we should error out with -ERESERVATION in any case.
Kevin
+ goto out; + }
+ if (vma->reserv_start <= cap_start && + vma->reserv_len >= cheri_length_get(cap)) + ret = true; +out: + if (!locked) + mmap_read_unlock(mm);
+ return ret; +}