On Mon, Sep 14, 2020 at 1:59 AM Jens Wiklander jens.wiklander@linaro.org wrote:
On Sun, Sep 13, 2020 at 10:12:11AM +0530, Souptick Joarder wrote:
When shm->num_pages <= 0, we should avoid calling release_registered_pages() in error handling path.
What are we fixing?
Current code is working fine and this patch is not needed. Sorry for the noise.
Signed-off-by: Souptick Joarder jrdr.linux@gmail.com Cc: John Hubbard jhubbard@nvidia.com
drivers/tee/tee_shm.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index 00472f5..e517d9f 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -260,8 +260,7 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, unsigned long addr, rc = get_kernel_pages(kiov, num_pages, 0, shm->pages); kfree(kiov); }
if (rc > 0)shm->num_pages = rc;
shm->num_pages = rc;Why not avoiding assigning invalid values to shm->num_pages? By the way, shm->num_pages is a size_t.
if (rc != num_pages) { if (rc >= 0) rc = -ENOMEM;@@ -309,7 +308,9 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, unsigned long addr, idr_remove(&teedev->idr, shm->id); mutex_unlock(&teedev->mutex); }
release_registered_pages(shm);
if (shm->pages && (shm->num_pages > 0))release_registered_pages(shm);With this we'll leak if shm->pages has been assigned something.
} kfree(shm); teedev_ctx_put(ctx);