diff options
author | Colin Cross <ccross@android.com> | 2013-12-20 19:43:50 -0500 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2014-02-07 17:21:09 -0500 |
commit | fee0c54e28f6ca187add93dfca226a8093cfa931 (patch) | |
tree | d62c8c58d90fda69cf035b36c457a8ce9df945ca | |
parent | 38dbfb59d1175ef458d006556061adeaa8751b72 (diff) |
dma-buf: avoid using IS_ERR_OR_NULL
dma_buf_map_attachment and dma_buf_vmap can return NULL or
ERR_PTR on a error. This encourages a common buggy pattern in
callers:
sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
if (IS_ERR_OR_NULL(sgt))
return PTR_ERR(sgt);
This causes the caller to return 0 on an error. IS_ERR_OR_NULL
is almost always a sign of poorly-defined error handling.
This patch converts dma_buf_map_attachment to always return
ERR_PTR, and fixes the callers that incorrectly handled NULL.
There are a few more callers that were not checking for NULL
at all, which would have dereferenced a NULL pointer later.
There are also a few more callers that correctly handled NULL
and ERR_PTR differently, I left those alone but they could also
be modified to delete the NULL check.
This patch also converts dma_buf_vmap to always return NULL.
All the callers to dma_buf_vmap only check for NULL, and would
have dereferenced an ERR_PTR and panic'd if one was ever
returned. This is not consistent with the rest of the dma buf
APIs, but matches the expectations of all of the callers.
Signed-off-by: Colin Cross <ccross@android.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r-- | drivers/base/dma-buf.c | 18 | ||||
-rw-r--r-- | drivers/gpu/drm/drm_prime.c | 2 | ||||
-rw-r--r-- | drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 2 | ||||
-rw-r--r-- | drivers/media/v4l2-core/videobuf2-dma-contig.c | 2 |
4 files changed, 14 insertions, 10 deletions
diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c index 1e16cbd61da2..cfe1d8bc7bb8 100644 --- a/drivers/base/dma-buf.c +++ b/drivers/base/dma-buf.c | |||
@@ -251,9 +251,8 @@ EXPORT_SYMBOL_GPL(dma_buf_put); | |||
251 | * @dmabuf: [in] buffer to attach device to. | 251 | * @dmabuf: [in] buffer to attach device to. |
252 | * @dev: [in] device to be attached. | 252 | * @dev: [in] device to be attached. |
253 | * | 253 | * |
254 | * Returns struct dma_buf_attachment * for this attachment; may return negative | 254 | * Returns struct dma_buf_attachment * for this attachment; returns ERR_PTR on |
255 | * error codes. | 255 | * error. |
256 | * | ||
257 | */ | 256 | */ |
258 | struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf, | 257 | struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf, |
259 | struct device *dev) | 258 | struct device *dev) |
@@ -319,9 +318,8 @@ EXPORT_SYMBOL_GPL(dma_buf_detach); | |||
319 | * @attach: [in] attachment whose scatterlist is to be returned | 318 | * @attach: [in] attachment whose scatterlist is to be returned |
320 | * @direction: [in] direction of DMA transfer | 319 | * @direction: [in] direction of DMA transfer |
321 | * | 320 | * |
322 | * Returns sg_table containing the scatterlist to be returned; may return NULL | 321 | * Returns sg_table containing the scatterlist to be returned; returns ERR_PTR |
323 | * or ERR_PTR. | 322 | * on error. |
324 | * | ||
325 | */ | 323 | */ |
326 | struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, | 324 | struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, |
327 | enum dma_data_direction direction) | 325 | enum dma_data_direction direction) |
@@ -334,6 +332,8 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, | |||
334 | return ERR_PTR(-EINVAL); | 332 | return ERR_PTR(-EINVAL); |
335 | 333 | ||
336 | sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction); | 334 | sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction); |
335 | if (!sg_table) | ||
336 | sg_table = ERR_PTR(-ENOMEM); | ||
337 | 337 | ||
338 | return sg_table; | 338 | return sg_table; |
339 | } | 339 | } |
@@ -544,6 +544,8 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap); | |||
544 | * These calls are optional in drivers. The intended use for them | 544 | * These calls are optional in drivers. The intended use for them |
545 | * is for mapping objects linear in kernel space for high use objects. | 545 | * is for mapping objects linear in kernel space for high use objects. |
546 | * Please attempt to use kmap/kunmap before thinking about these interfaces. | 546 | * Please attempt to use kmap/kunmap before thinking about these interfaces. |
547 | * | ||
548 | * Returns NULL on error. | ||
547 | */ | 549 | */ |
548 | void *dma_buf_vmap(struct dma_buf *dmabuf) | 550 | void *dma_buf_vmap(struct dma_buf *dmabuf) |
549 | { | 551 | { |
@@ -566,7 +568,9 @@ void *dma_buf_vmap(struct dma_buf *dmabuf) | |||
566 | BUG_ON(dmabuf->vmap_ptr); | 568 | BUG_ON(dmabuf->vmap_ptr); |
567 | 569 | ||
568 | ptr = dmabuf->ops->vmap(dmabuf); | 570 | ptr = dmabuf->ops->vmap(dmabuf); |
569 | if (IS_ERR_OR_NULL(ptr)) | 571 | if (WARN_ON_ONCE(IS_ERR(ptr))) |
572 | ptr = NULL; | ||
573 | if (!ptr) | ||
570 | goto out_unlock; | 574 | goto out_unlock; |
571 | 575 | ||
572 | dmabuf->vmap_ptr = ptr; | 576 | dmabuf->vmap_ptr = ptr; |
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 56805c39c906..bb516fdd195d 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c | |||
@@ -471,7 +471,7 @@ struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, | |||
471 | get_dma_buf(dma_buf); | 471 | get_dma_buf(dma_buf); |
472 | 472 | ||
473 | sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL); | 473 | sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL); |
474 | if (IS_ERR_OR_NULL(sgt)) { | 474 | if (IS_ERR(sgt)) { |
475 | ret = PTR_ERR(sgt); | 475 | ret = PTR_ERR(sgt); |
476 | goto fail_detach; | 476 | goto fail_detach; |
477 | } | 477 | } |
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c index 59827cc5e770..c786cd4f457b 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | |||
@@ -224,7 +224,7 @@ struct drm_gem_object *exynos_dmabuf_prime_import(struct drm_device *drm_dev, | |||
224 | get_dma_buf(dma_buf); | 224 | get_dma_buf(dma_buf); |
225 | 225 | ||
226 | sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL); | 226 | sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL); |
227 | if (IS_ERR_OR_NULL(sgt)) { | 227 | if (IS_ERR(sgt)) { |
228 | ret = PTR_ERR(sgt); | 228 | ret = PTR_ERR(sgt); |
229 | goto err_buf_detach; | 229 | goto err_buf_detach; |
230 | } | 230 | } |
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c index 33d3871d1e13..880be0782dd9 100644 --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c | |||
@@ -719,7 +719,7 @@ static int vb2_dc_map_dmabuf(void *mem_priv) | |||
719 | 719 | ||
720 | /* get the associated scatterlist for this buffer */ | 720 | /* get the associated scatterlist for this buffer */ |
721 | sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir); | 721 | sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir); |
722 | if (IS_ERR_OR_NULL(sgt)) { | 722 | if (IS_ERR(sgt)) { |
723 | pr_err("Error getting dmabuf scatterlist\n"); | 723 | pr_err("Error getting dmabuf scatterlist\n"); |
724 | return -EINVAL; | 724 | return -EINVAL; |
725 | } | 725 | } |