Skip to content

Commit c083a95

Browse files
committed
video-driver: fix sg_table leaks for internal meta buffers
The sg_table allocated for driver-owned buffers (buf->kvaddr != NULL) was never freed when the buffer was destroyed, causing the sg_table and its scatterlist entries to leak on every meta buffer teardown. Rename vb2_dc_get_base_sgt() to msm_vidc_get_sgtable and add msm_vidc_put_sgtable() to call sg_free_table(sg_table) and kfree(sgtable). Change-Id: I1b9e5c2707a73446702524aa593d701fb84595a8 Signed-off-by: Renjiang Han <renjiang@qti.qualcomm.com>
1 parent 8e46b4c commit c083a95

2 files changed

Lines changed: 36 additions & 6 deletions

File tree

driver/vidc/inc/msm_vidc_internal.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1082,6 +1082,8 @@ struct msm_vidc_buffer {
10821082
enum msm_vidc_buffer_attributes attr;
10831083
void *dmabuf;
10841084
struct sg_table *sg_table;
1085+
/* sg_table_alloc indicates whether sg_table allocated in video driver */
1086+
bool sg_table_alloc;
10851087
struct dma_buf_attachment *attach;
10861088
struct vb2_vmarea_handler handler;
10871089
refcount_t refcount;

driver/vidc/src/msm_vidc_vb2.c

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@
2626

2727
extern struct msm_vidc_core *g_core;
2828

29-
static struct sg_table *vb2_dc_get_base_sgt(struct msm_vidc_buffer *buf);
29+
static struct sg_table *msm_vidc_get_sg_table(struct msm_vidc_buffer *buf);
30+
static void msm_vidc_put_sg_table(struct msm_vidc_buffer *buf);
31+
3032
static void msm_vb2_vm_open(struct vm_area_struct *vma);
3133
static void msm_vb2_vm_close(struct vm_area_struct *vma);
3234
static void msm_vb2_put(void *buf_priv);
@@ -124,7 +126,7 @@ static void *msm_vb2_alloc(struct vb2_buffer *vb, struct device *dev,
124126
goto error_inst;
125127
}
126128

127-
buf->sg_table = vb2_dc_get_base_sgt(buf);
129+
buf->sg_table = msm_vidc_get_sg_table(buf);
128130
if (!buf->sg_table) {
129131
print_vidc_buffer(VIDC_ERR, "err ", "sg table build failed", inst, buf);
130132
rc = -ENOMEM;
@@ -152,8 +154,7 @@ static void *msm_vb2_alloc(struct vb2_buffer *vb, struct device *dev,
152154
return buf;
153155

154156
error_free_sgtable:
155-
kfree(buf->sg_table);
156-
buf->sg_table = NULL;
157+
msm_vidc_put_sg_table(buf);
157158
error_free_attrs:
158159
dma_free_attrs(cb->dev, buf->buffer_size, buf->kvaddr,
159160
buf->device_addr, buf->dma_attrs);
@@ -286,6 +287,13 @@ void msm_vb2_put(void *buf_priv)
286287
__func__, buf->device_addr, buf->buffer_size);
287288
dma_free_attrs(cb->dev, buf->buffer_size, buf->kvaddr,
288289
buf->device_addr, buf->dma_attrs);
290+
/* free the base sg_table allocated in msm_vb2_alloc via
291+
* msm_vidc_get_sg_table(); without this the sg_table and its
292+
* scatter list entries are leaked every time a meta buffer
293+
* is destroyed.
294+
*/
295+
if (buf->sg_table_alloc)
296+
msm_vidc_put_sg_table(buf);
289297
kfree(buf->dma_buf_info);
290298
buf->dma_buf_info = NULL;
291299
} else {
@@ -469,11 +477,13 @@ static void msm_vb2_dmabuf_ops_unmap(struct dma_buf_attachment *db_attach,
469477
static void msm_vb2_dmabuf_ops_release(struct dma_buf *dbuf)
470478
{
471479
struct msm_vidc_dma_buf_info *dinfo;
480+
struct msm_vidc_buffer *buf;
472481

473482
if (IS_ERR_OR_NULL(dbuf) || IS_ERR_OR_NULL(dbuf->priv))
474483
return;
475484

476485
dinfo = dbuf->priv;
486+
buf = dinfo->buf;
477487

478488
if (refcount_read(&dinfo->refcount) == 0)
479489
return;
@@ -489,6 +499,9 @@ static void msm_vb2_dmabuf_ops_release(struct dma_buf *dbuf)
489499
dma_free_attrs(dinfo->dev, dinfo->buffer_size, dinfo->kvaddr,
490500
dinfo->device_addr, dinfo->dma_attrs);
491501

502+
if (buf && buf->sg_table_alloc)
503+
msm_vidc_put_sg_table(buf);
504+
492505
kfree(dinfo);
493506
dbuf->priv = NULL;
494507
}
@@ -551,7 +564,7 @@ static const struct dma_buf_ops msm_vb2_dmabuf_ops = {
551564
.release = msm_vb2_dmabuf_ops_release,
552565
};
553566

554-
static struct sg_table *vb2_dc_get_base_sgt(struct msm_vidc_buffer *buf)
567+
static struct sg_table *msm_vidc_get_sg_table(struct msm_vidc_buffer *buf)
555568
{
556569
struct msm_vidc_inst *inst = buf->inst;
557570
struct msm_vidc_core *core = inst->core;
@@ -570,6 +583,7 @@ static struct sg_table *vb2_dc_get_base_sgt(struct msm_vidc_buffer *buf)
570583
cb = msm_vidc_get_context_bank_for_region(inst->core, region);
571584
if (!cb) {
572585
i_vpr_e(inst, "%s: failed to get context bank device\n", __func__);
586+
kfree(sgt);
573587
return NULL;
574588
}
575589

@@ -581,9 +595,23 @@ static struct sg_table *vb2_dc_get_base_sgt(struct msm_vidc_buffer *buf)
581595
return NULL;
582596
}
583597

598+
buf->sg_table_alloc = true;
584599
return sgt;
585600
}
586601

602+
static void msm_vidc_put_sg_table(struct msm_vidc_buffer *buf)
603+
{
604+
if (!buf->sg_table_alloc)
605+
return;
606+
607+
if (buf->sg_table) {
608+
sg_free_table(buf->sg_table);
609+
kfree(buf->sg_table);
610+
buf->sg_table = NULL;
611+
buf->sg_table_alloc = false;
612+
}
613+
}
614+
587615
static struct dma_buf *msm_vb2_get_dmabuf(struct vb2_buffer *vb,
588616
void *buf_priv,
589617
unsigned long flags)
@@ -603,7 +631,7 @@ static struct dma_buf *msm_vb2_get_dmabuf(struct vb2_buffer *vb,
603631
exp_info.priv = dinfo;
604632

605633
if (!buf->sg_table)
606-
buf->sg_table = vb2_dc_get_base_sgt(buf);
634+
buf->sg_table = msm_vidc_get_sg_table(buf);
607635

608636
if (WARN_ON(!buf->sg_table))
609637
return NULL;

0 commit comments

Comments
 (0)