From patchwork Sat Dec 21 00:43:50 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Colin Cross X-Patchwork-Id: 3392721 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 83D609F344 for ; Sat, 21 Dec 2013 00:44:48 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8E011206F6 for ; Sat, 21 Dec 2013 00:44:47 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 84781206F3 for ; Sat, 21 Dec 2013 00:44:46 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VuAg4-0000Sf-H4; Sat, 21 Dec 2013 00:44:24 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VuAg2-0003lr-9J; Sat, 21 Dec 2013 00:44:22 +0000 Received: from mail-qc0-f201.google.com ([209.85.216.201]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VuAfy-0003l4-BS for linux-arm-kernel@lists.infradead.org; Sat, 21 Dec 2013 00:44:19 +0000 Received: by mail-qc0-f201.google.com with SMTP id r5so203333qcx.0 for ; Fri, 20 Dec 2013 16:43:54 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=ViTZii9HHjqRDHRQ+/jOC4cV+PSFrz17DM8M4z3FUAE=; b=PrVRgvKbhUrkP4dBLtZXs/xOIO0N7yaol1Rdtx4ibhy3C2t6FbNLZnK16zDU2+VMde PdUrniT0V3GClfDrdagkWtNZOqVATOQ2fjToA8bZ5pDWZkBktpXSmXdNiTqxYNElclQ7 eFtNxuOeTGDe0aWbol9bYv3Z7NhZD6Z1zKQrAs9KSV6QBPGHjl3WUl+ZZqBwEgsxusbt ds7nlPNDz73JzIF1sxqzuFWhduAVkedQtyRCUn9mX/YS7QFPqX+FY9SkN3+ApUBqRxUq /vIQ0agNUl1+PwoBTAzlwJiB1JrRbDCkNTtME14nH8xrvPyvThuKl6Voq2vV1XvGbCi5 gJ7Q== X-Gm-Message-State: ALoCoQn3hC/9fi9BYMdF7O7ZBeYB1ahoLaKq3gG3NNOA7uAstq82sXFhGribGulZvFBBPwe8261x24uu3NMGIFIHSrj+h6CeQqPzztxOq6mJ6XAiA+fUhYCXMlYlv31OSph3JO0h3zPMX3LmdLKofB6/1QfK9yGhr0IqDVnKw2ddMa/eiHP4z9M/44Wjnw/x1u7fQzD1WIW3s1GQ8fSa+Of/K8XHwRBEvQ== X-Received: by 10.58.134.15 with SMTP id pg15mr949737veb.14.1387586634745; Fri, 20 Dec 2013 16:43:54 -0800 (PST) Received: from corp2gmr1-1.hot.corp.google.com (corp2gmr1-1.hot.corp.google.com [172.24.189.92]) by gmr-mx.google.com with ESMTPS id o30si3308065yhn.1.2013.12.20.16.43.54 for (version=TLSv1.1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 20 Dec 2013 16:43:54 -0800 (PST) Received: from walnut.mtv.corp.google.com (walnut.mtv.corp.google.com [172.18.120.100]) by corp2gmr1-1.hot.corp.google.com (Postfix) with ESMTP id 8000431C19E; Fri, 20 Dec 2013 16:43:54 -0800 (PST) Received: by walnut.mtv.corp.google.com (Postfix, from userid 99897) id 18714160888; Fri, 20 Dec 2013 16:43:53 -0800 (PST) From: Colin Cross To: linux-kernel@vger.kernel.org Subject: [PATCH] dma-buf: avoid using IS_ERR_OR_NULL Date: Fri, 20 Dec 2013 16:43:50 -0800 Message-Id: <1387586630-1954-1-git-send-email-ccross@android.com> X-Mailer: git-send-email 1.8.5.1 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20131220_194418_479109_8E487561 X-CRM114-Status: GOOD ( 16.60 ) X-Spam-Score: -3.1 (---) Cc: "open list:DMA BUFFER SHARIN..." , Kukjin Kim , Joonyoung Shim , Pawel Osciak , David Airlie , Greg Kroah-Hartman , Seung-Woo Kim , "open list:DMA BUFFER SHARIN..." , "open list:DMA BUFFER SHARIN..." , Inki Dae , Kyungmin Park , "moderated list:ARM/S5P EXYNOS AR..." , Colin Cross , Mauro Carvalho Chehab , Sumit Semwal , "moderated list:ARM/S5P EXYNOS AR..." , Marek Szyprowski X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 Reviewed-by: Rob Clark Reviewed-by: Mauro Carvalho Chehab --- drivers/base/dma-buf.c | 18 +++++++++++------- drivers/gpu/drm/drm_prime.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 2 +- 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); * @dmabuf: [in] buffer to attach device to. * @dev: [in] device to be attached. * - * Returns struct dma_buf_attachment * for this attachment; may return negative - * error codes. - * + * Returns struct dma_buf_attachment * for this attachment; returns ERR_PTR on + * error. */ struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf, struct device *dev) @@ -319,9 +318,8 @@ EXPORT_SYMBOL_GPL(dma_buf_detach); * @attach: [in] attachment whose scatterlist is to be returned * @direction: [in] direction of DMA transfer * - * Returns sg_table containing the scatterlist to be returned; may return NULL - * or ERR_PTR. - * + * Returns sg_table containing the scatterlist to be returned; returns ERR_PTR + * on error. */ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, enum dma_data_direction direction) @@ -334,6 +332,8 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, return ERR_PTR(-EINVAL); sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction); + if (!sg_table) + sg_table = ERR_PTR(-ENOMEM); return sg_table; } @@ -544,6 +544,8 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap); * These calls are optional in drivers. The intended use for them * is for mapping objects linear in kernel space for high use objects. * Please attempt to use kmap/kunmap before thinking about these interfaces. + * + * Returns NULL on error. */ void *dma_buf_vmap(struct dma_buf *dmabuf) { @@ -566,7 +568,9 @@ void *dma_buf_vmap(struct dma_buf *dmabuf) BUG_ON(dmabuf->vmap_ptr); ptr = dmabuf->ops->vmap(dmabuf); - if (IS_ERR_OR_NULL(ptr)) + if (WARN_ON_ONCE(IS_ERR(ptr))) + ptr = NULL; + if (!ptr) goto out_unlock; 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, get_dma_buf(dma_buf); sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL); - if (IS_ERR_OR_NULL(sgt)) { + if (IS_ERR(sgt)) { ret = PTR_ERR(sgt); goto fail_detach; } 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, get_dma_buf(dma_buf); sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL); - if (IS_ERR_OR_NULL(sgt)) { + if (IS_ERR(sgt)) { ret = PTR_ERR(sgt); goto err_buf_detach; } 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) /* get the associated scatterlist for this buffer */ sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir); - if (IS_ERR_OR_NULL(sgt)) { + if (IS_ERR(sgt)) { pr_err("Error getting dmabuf scatterlist\n"); return -EINVAL; }