| Message ID | 20211007232450.2036087-1-volodymyr_babchuk@epam.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: optee: fix possible memory leaks | expand |
On Thu, 7 Oct 2021, Volodymyr Babchuk wrote: > translate_noncontig() allocates domheap page for translated list > before calling to allocate_optee_shm_buf(), which can fail for number > of reason. Anyways, after fail we need to free the allocated page(s). > > Another leak is possible if the same translate_noncontig() function > fails to get domain page. In this case it should free allocated > optee_shm_buf prior exit. This will also free allocated domheap page. > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> > --- > xen/arch/arm/tee/optee.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c > index 6df0d44eb9..131d2f9a8a 100644 > --- a/xen/arch/arm/tee/optee.c > +++ b/xen/arch/arm/tee/optee.c > @@ -781,7 +781,10 @@ static int translate_noncontig(struct optee_domain *ctx, > optee_shm_buf = allocate_optee_shm_buf(ctx, param->u.tmem.shm_ref, > pg_count, xen_pgs, order); > if ( IS_ERR(optee_shm_buf) ) > + { > + free_domheap_pages(xen_pgs, order); > return PTR_ERR(optee_shm_buf); > + } > > gfn = gaddr_to_gfn(param->u.tmem.buf_ptr & > ~(OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1)); > @@ -807,7 +810,7 @@ static int translate_noncontig(struct optee_domain *ctx, > { > guest_pg = get_domain_ram_page(gfn); > if ( !guest_pg ) > - return -EINVAL; > + goto free_shm_buf; > > guest_data = __map_domain_page(guest_pg); > xen_data = __map_domain_page(xen_pgs); > @@ -854,6 +857,7 @@ err_unmap: > unmap_domain_page(guest_data); > unmap_domain_page(xen_data); > put_page(guest_pg); > +free_shm_buf: > free_optee_shm_buf(ctx, optee_shm_buf->cookie); > > return -EINVAL; > -- > 2.33.0 >
diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c index 6df0d44eb9..131d2f9a8a 100644 --- a/xen/arch/arm/tee/optee.c +++ b/xen/arch/arm/tee/optee.c @@ -781,7 +781,10 @@ static int translate_noncontig(struct optee_domain *ctx, optee_shm_buf = allocate_optee_shm_buf(ctx, param->u.tmem.shm_ref, pg_count, xen_pgs, order); if ( IS_ERR(optee_shm_buf) ) + { + free_domheap_pages(xen_pgs, order); return PTR_ERR(optee_shm_buf); + } gfn = gaddr_to_gfn(param->u.tmem.buf_ptr & ~(OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1)); @@ -807,7 +810,7 @@ static int translate_noncontig(struct optee_domain *ctx, { guest_pg = get_domain_ram_page(gfn); if ( !guest_pg ) - return -EINVAL; + goto free_shm_buf; guest_data = __map_domain_page(guest_pg); xen_data = __map_domain_page(xen_pgs); @@ -854,6 +857,7 @@ err_unmap: unmap_domain_page(guest_data); unmap_domain_page(xen_data); put_page(guest_pg); +free_shm_buf: free_optee_shm_buf(ctx, optee_shm_buf->cookie); return -EINVAL;
translate_noncontig() allocates domheap page for translated list before calling to allocate_optee_shm_buf(), which can fail for number of reason. Anyways, after fail we need to free the allocated page(s). Another leak is possible if the same translate_noncontig() function fails to get domain page. In this case it should free allocated optee_shm_buf prior exit. This will also free allocated domheap page. Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> --- xen/arch/arm/tee/optee.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)