On Fri, Jul 17, 2026 at 11:10:15AM +0000, [email protected] wrote:
> > +static void cla_vma_open(struct vm_area_struct *vma)
> > +{
> > + /*
> > + * A vma previously created with cla_file_mmap() has been duplicated
> > + * within the same mm (most likely due to mremap). While this could also
> > + * be called for duplication into a new mm (via fork), we set
> > + * VM_DONTCOPY on the original mmap, so this will never happen. So the
> > + * contexts covered by this new vma already exist.
> > + */
> > + struct cla_domain *domain;
> > +
> > + cla_for_each_mapped_domain(vma, domain)
> > + cla_ctx_map(domain, vma->vm_mm, vma->vm_file);
>
> [Severity: High]
> Does this code need to check the return value of cla_ctx_map()?
>
> If a VMA is duplicated (e.g., via mremap with MREMAP_MAYMOVE) and the
> domain is in a broken state, cla_ctx_map() will return ERR_PTR(-EIO)
> without incrementing the context refcount.
>
> Because cla_vma_open() silently ignores this error, the refcount remains
> unbalanced. When the old VMA is subsequently closed, cla_ctx_unmap() drops
> the refcount to zero and frees the context.
I think it wouldn't hurt to move the domain->broken check after
cla_domain_lookup_ctx()
> > +static vm_fault_t cla_vma_fault(struct vm_fault *vmf)
> > +{
> > + struct vm_area_struct *vma = vmf->vma;
> > + struct cla_domain *domain;
> > + struct cla_dev *dev;
> > + struct cla_ctx *ctx;
> > + unsigned long pg;
> > +
> > + /*
> > + * Reassignment needs mmap_lock, so we cannot wait while holding it.
> > + * Luckily, this flag is only missing in some exotic cases which do not
> > + * apply for this VM_IO | VM_PFNMAP vma. i.e. GUP without
> > + * FOLL_UNLOCKABLE or FOLL_NOWAIT or fixup_user_fault(unlocked=NULL),
> > + * used by futex fault-in path.
> > + */
> > + if (WARN_ON(!(vmf->flags & FAULT_FLAG_ALLOW_RETRY)))
> > + return VM_FAULT_SIGBUS;
>
> [Severity: High]
> Is it safe to unconditionally assume FAULT_FLAG_ALLOW_RETRY is set here?
I'll have a look myself at some point, but the experts say it is fine.
> Under the per-VMA lock fault path (which is the default for modern kernels),
> FAULT_FLAG_VMA_LOCK is set but FAULT_FLAG_ALLOW_RETRY is explicitly absent.
>
> Consequently, any normal page fault handled under the VMA lock will hit this
> branch, returning VM_FAULT_SIGBUS and breaking normal memory access.
>
> Additionally, because this is user-controllable, could the WARN_ON allow an
> unprivileged local attacker to spam the kernel log or crash the system if
> panic_on_warn is enabled?
