DMA handles passed as invoke arguments (scalars beyond nbufs) may refer
to the same dma_buf fd as an input/output buffer argument. Taking an
extra reference for such DMA handle maps leads to duplicate mappings and
an unbalanced reference count, since DMA handle maps are released
separately when the DSP returns the fd through the fdlist.
Fix this by not taking an extra reference for DMA handle arguments
(take_ref = false) and tagging them with FASTRPC_MAP_DMA_HANDLE. As
these maps are borrowed references, fastrpc_get_args() re-validates the
map via fastrpc_map_lookup() before dereferencing it, so it is not used
after being freed. fastrpc_put_args() only releases maps flagged as
FASTRPC_MAP_DMA_HANDLE and clears the flag to guarantee the map is freed
exactly once.
Fixes: 10df039834f84 ("misc: fastrpc: Skip reference for DMA handles")
Cc: [email protected]
Signed-off-by: Jianping Li <[email protected]>
---
Patch [v3]:
https://lore.kernel.org/all/[email protected]/
Changes in v4:
- Do not expose FASTRPC_MAP_DMA_HANDLE through the uapi header. The flag
is only set and consumed by the driver, userspace never passes it as an
input flag, so define it privately in fastrpc.c instead. This also keeps
FASTRPC_MAP_MAX as the real upper bound of the uapi flags.
- Drop the FASTRPC_MAP_DMA_HANDLE rejection in fastrpc_req_mem_map(): with
the flag no longer in the uapi, userspace cannot pass this value through
the MEM_MAP path, so the check is dead code. The commit message paragraph
describing it is removed as well.
Changes in v3:
- No functional changes.
- fastrpc_put_args(): document that clearing map->flags without a lock
is safe because the DSP reports a given fd in the fdlist only once,
so no concurrent fastrpc_put_args() can race on the same map's flags.
Changes in v2:
- Rework the commit message to describe the DMA handle reference and
lifetime problem more precisely.
- Introduce a new FASTRPC_MAP_DMA_HANDLE uapi flag and a 'flags' field
in struct fastrpc_map to explicitly tag DMA handle maps, instead of
relying only on the nbufs boundary / take_ref.
- Plumb an mflags argument through fastrpc_map_create() and
fastrpc_map_attach() so DMA handle maps are tagged at creation time.
- Re-validate the borrowed map in fastrpc_get_args() via
fastrpc_map_lookup() before dereferencing it, to avoid a
use-after-free when the map was created with take_ref = false.
- In fastrpc_put_args(), only release maps tagged FASTRPC_MAP_DMA_HANDLE
and clear the flag afterwards, so such maps are freed exactly once.
- Reject FASTRPC_MAP_DMA_HANDLE in fastrpc_req_mem_map(), since these
handles are already mapped implicitly during the remote invoke and
must not be mapped again through the explicit MEM_MAP path.
---
drivers/misc/fastrpc.c | 56 +++++++++++++++++++++++++++++++-----------
1 file changed, 41 insertions(+), 15 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index d4fac2caca86..58f27e271317 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -52,6 +52,9 @@
#define FASTRPC_CTXID_SEQ_SHIFT 16
#define FASTRPC_CTXID_SEQ_MASK GENMASK_ULL(63, 16)
+/* Map the DMA handle in the invoke call for backward compatibility */
+#define FASTRPC_MAP_DMA_HANDLE 0x20000
+
/*
* Newer DSP firmware implements a PD (Protection Domain) notification
* framework that sends PD state notifications upon request. The PD exit
@@ -253,6 +256,7 @@ struct fastrpc_map {
u64 len;
u64 raddr;
u32 attr;
+ u32 flags;
struct kref refcount;
};
@@ -879,7 +883,7 @@ static dma_addr_t fastrpc_compute_dma_addr(struct
fastrpc_user *fl, dma_addr_t s
}
static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
- u64 len, u32 attr, struct fastrpc_map **ppmap)
+ u64 len, u32 attr, struct fastrpc_map **ppmap,
int mflags)
{
struct fastrpc_session_ctx *sess = fl->sctx;
struct fastrpc_map *map = NULL;
@@ -896,6 +900,7 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int
fd,
map->fl = fl;
map->fd = fd;
+ map->flags = mflags;
map->buf = dma_buf_get(fd);
if (IS_ERR(map->buf)) {
err = PTR_ERR(map->buf);
@@ -970,13 +975,13 @@ static int fastrpc_map_attach(struct fastrpc_user *fl,
int fd,
return err;
}
-static int fastrpc_map_create(struct fastrpc_user *fl, int fd,
- u64 len, u32 attr, struct fastrpc_map **ppmap)
+static int fastrpc_map_create(struct fastrpc_user *fl, int fd, u64 len, u32
attr,
+ struct fastrpc_map **ppmap, bool take_ref, int
mflags)
{
- if (!fastrpc_map_lookup(fl, fd, ppmap, true))
+ if (!fastrpc_map_lookup(fl, fd, ppmap, take_ref))
return 0;
- return fastrpc_map_attach(fl, fd, len, attr, ppmap);
+ return fastrpc_map_attach(fl, fd, len, attr, ppmap, mflags);
}
/*
@@ -1047,23 +1052,25 @@ static int fastrpc_create_maps(struct
fastrpc_invoke_ctx *ctx)
int i, err;
for (i = 0; i < ctx->nscalars; ++i) {
+ bool take_ref = i < ctx->nbufs;
+ int mflags = 0;
if (ctx->args[i].fd == 0 || ctx->args[i].fd == -1 ||
ctx->args[i].length == 0)
continue;
- if (i < ctx->nbufs)
- err = fastrpc_map_create(ctx->fl, ctx->args[i].fd,
- ctx->args[i].length, ctx->args[i].attr,
&ctx->maps[i]);
- else
- err = fastrpc_map_attach(ctx->fl, ctx->args[i].fd,
- ctx->args[i].length, ctx->args[i].attr,
&ctx->maps[i]);
+ /* Set the DMA handle mapping flag for DMA handles */
+ if (i >= ctx->nbufs)
+ mflags = FASTRPC_MAP_DMA_HANDLE;
+
+ err = fastrpc_map_create(ctx->fl, ctx->args[i].fd,
ctx->args[i].length,
+ ctx->args[i].attr, &ctx->maps[i],
take_ref, mflags);
if (err) {
dev_err(dev, "Error Creating map %d\n", err);
return -EINVAL;
}
-
}
+
return 0;
}
@@ -1195,6 +1202,16 @@ static int fastrpc_get_args(u32 kernel, struct
fastrpc_invoke_ctx *ctx)
list[i].num = ctx->args[i].length ? 1 : 0;
list[i].pgidx = i;
if (ctx->maps[i]) {
+ /* It is possible that map is created with
+ * mflags FASTRPC_MAP_DMA_HANDLE and take_ref
+ * is false. Check if map still exists or is
+ * being freed as take_ref is false
+ */
+ if (fastrpc_map_lookup(ctx->fl, ctx->args[i].fd,
+ &ctx->maps[i], false)) {
+ ctx->maps[i] = NULL;
+ return -EINVAL;
+ }
pages[i].addr = ctx->maps[i]->dma_addr;
pages[i].size = ctx->maps[i]->size;
}
@@ -1244,8 +1261,17 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx
*ctx,
for (i = 0; i < FASTRPC_MAX_FDLIST; i++) {
if (!fdlist[i])
break;
- if (!fastrpc_map_lookup(fl, (int)fdlist[i], &mmap, false))
+ /*
+ * DMA handle maps are released when the DSP returns the
corresponding fd in
+ * fdlist. The DSP is expected to return a specific fd only
once in fdlist,
+ * so no two fastrpc_put_args() paths should clear the
DMA_HANDLE flag for
+ * the same map concurrently.
+ */
+ if (!fastrpc_map_lookup(fl, (int)fdlist[i], &mmap, false) &&
+ mmap->flags == FASTRPC_MAP_DMA_HANDLE) {
+ mmap->flags = 0;
fastrpc_map_put(mmap);
+ }
}
return ret;
@@ -1621,7 +1647,7 @@ static int fastrpc_init_create_process(struct
fastrpc_user *fl,
fl->pd = USER_PD;
if (init.filelen && init.filefd) {
- err = fastrpc_map_create(fl, init.filefd, init.filelen, 0,
&map);
+ err = fastrpc_map_create(fl, init.filefd, init.filelen, 0,
&map, true, 0);
if (err)
goto err;
}
@@ -2244,7 +2270,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl,
char __user *argp)
return -EFAULT;
/* create SMMU mapping */
- err = fastrpc_map_create(fl, req.fd, req.length, 0, &map);
+ err = fastrpc_map_create(fl, req.fd, req.length, 0, &map, true, 0);
if (err) {
dev_err(dev, "failed to map buffer, fd = %d\n", req.fd);
return err;
--
2.43.0
