Re: [PATCH v3] misc: fastrpc: Fix double free and races in fastrpc_req_mmap() error path

[Yao Yiqi]

On Sep 30th, Sashiko AI wrote:
> [Severity: Critical]
> Is there a risk of a DMA use-after-free if the unmap operation is
> interrupted by a signal?
> ...
> If fastrpc_req_munmap_impl() returns -ERESTARTSYS, this err_assign path
> then manually adds the buffer back to fl->mmaps. When the process later
> exits, fastrpc_user_free() will reclaim this buffer while the DSP may
> still be using it, which could corrupt newly allocated kernel memory.

Thanks for the review.  This path is not reachable.

fastrpc_req_munmap_impl() issues the unmap RPC with kernel = true:

	err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc,
				      &args[0]);

and fastrpc_internal_invoke() waits differently depending on that flag
(fastrpc_wait_for_response(), drivers/misc/fastrpc.c:1320):

	if (kernel) {
		if (!wait_for_completion_timeout(&ctx->work, 10 * HZ))
			err = -ETIMEDOUT;
	} else {
		err = wait_for_completion_interruptible(&ctx->work);
	}

wait_for_completion_timeout() is uninterruptible, so the kernel invoke can
only return 0 or -ETIMEDOUT, never -ERESTARTSYS.  The interruptible wait
above is the only source of -ERESTARTSYS in this driver, and it is only
used for user invokes (FASTRPC_IOCTL_INVOKE).  Therefore the
interrupted-invoke cleanup you quoted (fastrpc.c:1418) never runs for the
unmap RPC, no matter whether the buffer is linked into fl->mmaps at that
point, and fastrpc_req_munmap_impl() cannot return -ERESTARTSYS from the
err_assign path.

For the failure that the unmap RPC can actually return (-ETIMEDOUT or a
DSP error), re-adding the buffer to fl->mmaps keeps it tracked so that
fastrpc_user_free() reclaims it on release.  That is exactly what
fastrpc_req_munmap() has done since 6102ceb4eab8 ("misc: fastrpc: Remove
buffer from list prior to unmap operation") for a failed unmap, and this
patch deliberately preserves that existing behaviour.

Best regards,
Yiqi