| Message ID | 20211008062903.39731-1-guangming.cao@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dma-buf: acquire name lock before read/write dma_buf.name | expand |
Am 08.10.21 um 08:29 schrieb guangming.cao@mediatek.com: > From: Guangming Cao <Guangming.Cao@mediatek.com> > > Because dma-buf.name can be freed in func: "dma_buf_set_name", > so, we need to acquire lock first before we read/write dma_buf.name > to prevent Use After Free(UAF) issue. > > Signed-off-by: Guangming Cao <Guangming.Cao@mediatek.com> > --- > drivers/dma-buf/dma-buf.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index 511fe0d217a0..aebb51b3ff52 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -80,7 +80,9 @@ static void dma_buf_release(struct dentry *dentry) > dma_resv_fini(dmabuf->resv); > > module_put(dmabuf->owner); > + spin_lock(&dmabuf->name_lock); > kfree(dmabuf->name); > + spin_unlock(&dmabuf->name_lock); That here is certainly a NAK. This is the release function if somebody is changing the name on a released DMA-buf we have much bigger problems. > kfree(dmabuf); > } > > @@ -1372,6 +1374,8 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused) > if (ret) > goto error_unlock; > > + > + spin_lock(&dmabuf->name_lock); > seq_printf(s, "%08zu\t%08x\t%08x\t%08ld\t%s\t%08lu\t%s\n", > buf_obj->size, > buf_obj->file->f_flags, buf_obj->file->f_mode, > @@ -1379,6 +1383,7 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused) > buf_obj->exp_name, > file_inode(buf_obj->file)->i_ino, > buf_obj->name ?: ""); > + spin_unlock(&dmabuf->name_lock); Yeah, that part looks like a good idea to me as well. Christian. > > robj = buf_obj->resv; > fence = dma_resv_excl_fence(robj);
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 511fe0d217a0..aebb51b3ff52 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -80,7 +80,9 @@ static void dma_buf_release(struct dentry *dentry) dma_resv_fini(dmabuf->resv); module_put(dmabuf->owner); + spin_lock(&dmabuf->name_lock); kfree(dmabuf->name); + spin_unlock(&dmabuf->name_lock); kfree(dmabuf); } @@ -1372,6 +1374,8 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused) if (ret) goto error_unlock; + + spin_lock(&dmabuf->name_lock); seq_printf(s, "%08zu\t%08x\t%08x\t%08ld\t%s\t%08lu\t%s\n", buf_obj->size, buf_obj->file->f_flags, buf_obj->file->f_mode, @@ -1379,6 +1383,7 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused) buf_obj->exp_name, file_inode(buf_obj->file)->i_ino, buf_obj->name ?: ""); + spin_unlock(&dmabuf->name_lock); robj = buf_obj->resv; fence = dma_resv_excl_fence(robj);