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?
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); -- 1.9.1
Thanks, Jens