Thanks Dan for reporting this,
On 11/05/2020 15:51, Dan Carpenter wrote:
Hello Srinivas Kandagatla,
The patch c68cfb718c8f: "misc: fastrpc: Add support for context
Invoke method" from Feb 8, 2019, leads to the following static
checker warning:
drivers/misc/fastrpc.c:990 fastrpc_internal_invoke()
warn: 'ctx->refcount.refcount.ref.counter' not decremented on lines: 990.
drivers/misc/fastrpc.c
925 static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel,
926 u32 handle, u32 sc,
927 struct fastrpc_invoke_args *args)
928 {
929 struct fastrpc_invoke_ctx *ctx = NULL;
930 int err = 0;
931
932 if (!fl->sctx)
933 return -EINVAL;
934
935 if (!fl->cctx->rpdev)
936 return -EPIPE;
937
938 ctx = fastrpc_context_alloc(fl, kernel, sc, args);
refcount is 1.
939 if (IS_ERR(ctx))
940 return PTR_ERR(ctx);
941
942 if (ctx->nscalars) {
943 err = fastrpc_get_args(kernel, ctx);
944 if (err)
945 goto bail;
^^^^^^^^^
Still holding one refcount.
946 }
947
948 /* make sure that all CPU memory writes are seen by DSP */
949 dma_wmb();
950 /* Send invoke buffer to remote dsp */
951 err = fastrpc_invoke_send(fl->sctx, ctx, kernel, handle);
^^^^^^^^^^^^^^^^^^^
Takes a reference count. Refcount is now 2.
952 if (err)
953 goto bail;
954
955 if (kernel) {
956 if (!wait_for_completion_timeout(&ctx->work, 10 * HZ))
957 err = -ETIMEDOUT;
958 } else {
959 err = wait_for_completion_interruptible(&ctx->work);
This drops a refcount.
960 }
961
962 if (err)
963 goto bail;
964
965 /* Check the response from remote dsp */
966 err = ctx->retval;
967 if (err)
968 goto bail;
969
970 if (ctx->nscalars) {
971 /* make sure that all memory writes by DSP are seen by CPU */
972 dma_rmb();
973 /* populate all the output buffers with results */
974 err = fastrpc_put_args(ctx, kernel);
975 if (err)
976 goto bail;
977 }
978
979 bail:
980 if (err != -ERESTARTSYS && err != -ETIMEDOUT) {
981 /* We are done with this compute context */
982 spin_lock(&fl->lock);
983 list_del(&ctx->node);
984 spin_unlock(&fl->lock);
985 fastrpc_context_put(ctx);
If we're holding two refcounts then I think this leaks.
Code can enter with two refcounts only an error other than -ERESTARTSYS
&& err != -ETIMEDOUT. This is only possible rpmsg_send() fails for some
reason.
Let me send a patch to fix this!
thanks,
srini
986 }
987 if (err)
988 dev_dbg(fl->sctx->dev, "Error: Invoke Failed %d\n", err);
989
990 return err;
991 }
regards,
dan carpenter
