From 19cf289669e8dc5f76f8a562339f7d64c9ed2cf6 Mon Sep 17 00:00:00 2001 From: Mitchel Humpherys Date: Thu, 12 Feb 2015 21:28:07 -0800 Subject: [PATCH] ion: return a fresh sg_table from ion_map_dma_buf DMA mappers often (always?) modify the scatterlist they are given (e.g. dma_address and dma_length) when their dma_map_ops.map_sg operation is called. Currently Ion always returns the same sg_table when it's dma_buf_ops.map_dma_buf operation is called. The result is that when Ion is used as a DMA buf exporter to multiple clients (a common scenario during zero-copy buffer-sharing) those clients will be stomping all over each other since they're both trying to put their mappings into the same sg_table. Fix this by always duplicating the buffer's sg_table in ion_map_dma_buf and returning the duplicate. Each buffer will still always have a single "master" sg_table that should never be exported to the DMA buf framework. Test case (pseudo-code): handle = ion_alloc(); dbuf = ion_share_dma_buf(handle); attach = dma_buf_attach(dbuf); table1 = dma_buf_map_attachment(attach); table2 = dma_buf_map_attachment(attach); dma_map_sg(table1); table2 = dma_buf_map_attachment(attach); dma_map_sg(table2); /* table1 is now hosed */ check_mappings(table1); /* KABOOM */ Change-Id: Ifefa06ecd464c00819b4ae7ac32ad37f1491373b Signed-off-by: Mitchel Humpherys --- drivers/staging/android/ion/ion.c | 40 +++++++++++++++++++++++++- drivers/staging/android/ion/ion_priv.h | 6 +++- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index d5d019d3a0b6..dda24f21d18f 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -949,6 +949,13 @@ int ion_handle_get_size(struct ion_client *client, struct ion_handle *handle, } EXPORT_SYMBOL(ion_handle_get_size); +/** + * ion_sg_table - get an sg_table for the buffer + * + * NOTE: most likely you should NOT being using this API. + * You should be using Ion as a DMA Buf exporter and using + * the sg_table returned by dma_buf_map_attachment. + */ struct sg_table *ion_sg_table(struct ion_client *client, struct ion_handle *handle) { @@ -999,6 +1006,30 @@ err0: return ERR_PTR(ret); } +static struct sg_table *ion_dupe_sg_table(struct sg_table *orig_table) +{ + int ret, i; + struct scatterlist *sg, *sg_orig; + struct sg_table *table; + + table = kzalloc(sizeof(struct sg_table), GFP_KERNEL); + if (!table) + return NULL; + + ret = sg_alloc_table(table, orig_table->nents, GFP_KERNEL); + if (ret) { + kfree(table); + return NULL; + } + + sg_orig = orig_table->sgl; + for_each_sg(table->sgl, sg, table->nents, i) { + memcpy(sg, sg_orig, sizeof(*sg)); + sg_orig = sg_next(sg_orig); + } + return table; +} + static void ion_buffer_sync_for_device(struct ion_buffer *buffer, struct device *dev, enum dma_data_direction direction); @@ -1008,15 +1039,22 @@ static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment, { struct dma_buf *dmabuf = attachment->dmabuf; struct ion_buffer *buffer = dmabuf->priv; + struct sg_table *table; + + table = ion_dupe_sg_table(buffer->sg_table); + if (!table) + return NULL; ion_buffer_sync_for_device(buffer, attachment->dev, direction); - return buffer->sg_table; + return table; } static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment, struct sg_table *table, enum dma_data_direction direction) { + sg_free_table(table); + kfree(table); } void ion_pages_sync_for_device(struct device *dev, struct page *page, diff --git a/drivers/staging/android/ion/ion_priv.h b/drivers/staging/android/ion/ion_priv.h index f7827e2c334c..274892fed721 100644 --- a/drivers/staging/android/ion/ion_priv.h +++ b/drivers/staging/android/ion/ion_priv.h @@ -55,7 +55,11 @@ struct ion_buffer *ion_handle_buffer(struct ion_handle *handle); * @lock: protects the buffers cnt fields * @kmap_cnt: number of times the buffer is mapped to the kernel * @vaddr: the kenrel mapping if kmap_cnt is not zero - * @sg_table: the sg table for the buffer + * @sg_table: the sg table for the buffer. Note that if you need + * an sg_table for this buffer, you should likely be + * using Ion as a DMA Buf exporter and using + * dma_buf_map_attachment rather than trying to use this + * field directly. * @pages: flat array of pages in the buffer -- used by fault * handler and only valid for buffers that are faulted in * @vmas: list of vma's mapping this buffer