From 36b347fb31e74075315a782809007b9192c486ea Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sat, 5 Apr 2014 10:03:22 +0200 Subject: [PATCH 01/12] drm/mgag200: Remove unecessary NULL check in bo_unref ttm_bo_unref unconditionally calls kref_put on it's argument, so the thing can't be NULL without already causing Oopses. Spotted by coverity. Reviewed-by: David Herrmann Reviewed-by: Ian Romanick Signed-off-by: Daniel Vetter --- drivers/gpu/drm/mgag200/mgag200_main.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c index 26868e5c55b0..0722d18992f4 100644 --- a/drivers/gpu/drm/mgag200/mgag200_main.c +++ b/drivers/gpu/drm/mgag200/mgag200_main.c @@ -322,9 +322,7 @@ static void mgag200_bo_unref(struct mgag200_bo **bo) tbo = &((*bo)->bo); ttm_bo_unref(&tbo); - if (tbo == NULL) - *bo = NULL; - + *bo = NULL; } void mgag200_gem_free_object(struct drm_gem_object *obj) From 4cb8802e28c14a4b34827154ab1537bbde796cc2 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sat, 5 Apr 2014 10:06:16 +0200 Subject: [PATCH 02/12] drm/mgag200: Remove unecessary NULL check in gem_free The ->gem_free_object never gets called with a NULL pointer, the check is redundant. Also checking after the upcast allows compilers to elide it anyway. Spotted by coverity. Reviewed-by: David Herrmann Reviewed-by: Ian Romanick Signed-off-by: Daniel Vetter --- drivers/gpu/drm/mgag200/mgag200_main.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c index 0722d18992f4..f6b283b8375e 100644 --- a/drivers/gpu/drm/mgag200/mgag200_main.c +++ b/drivers/gpu/drm/mgag200/mgag200_main.c @@ -329,8 +329,6 @@ void mgag200_gem_free_object(struct drm_gem_object *obj) { struct mgag200_bo *mgag200_bo = gem_to_mga_bo(obj); - if (!mgag200_bo) - return; mgag200_bo_unref(&mgag200_bo); } From 275c63222761255e5a34eac8327ea65a10fdd8d4 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sat, 5 Apr 2014 10:09:36 +0200 Subject: [PATCH 03/12] drm/cirrus: Remove unnecessary NULL check in bo_unref ttm_bo_unref unconditionally calls kref_put on it's argument, so the thing can't be NULL without already causing Oopses. Spotted by coverity. Reviewed-by: David Herrmann Reviewed-by: Ian Romanick Signed-off-by: Daniel Vetter --- drivers/gpu/drm/cirrus/cirrus_main.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c b/drivers/gpu/drm/cirrus/cirrus_main.c index 4b0170cf53fd..b119f66fed92 100644 --- a/drivers/gpu/drm/cirrus/cirrus_main.c +++ b/drivers/gpu/drm/cirrus/cirrus_main.c @@ -264,9 +264,7 @@ static void cirrus_bo_unref(struct cirrus_bo **bo) tbo = &((*bo)->bo); ttm_bo_unref(&tbo); - if (tbo == NULL) - *bo = NULL; - + *bo = NULL; } void cirrus_gem_free_object(struct drm_gem_object *obj) From 3d535eddf2b7fc93126aaf4e36c05fe751fb8c4b Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sat, 5 Apr 2014 10:10:08 +0200 Subject: [PATCH 04/12] drm/cirrus: Remove unecessary NULL check in gem_free The ->gem_free_object never gets called with a NULL pointer, the check is redundant. Also checking after the upcast allows compilers to elide it anyway. Spotted by coverity. Reviewed-by: David Herrmann Reviewed-by: Ian Romanick Signed-off-by: Daniel Vetter --- drivers/gpu/drm/cirrus/cirrus_main.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c b/drivers/gpu/drm/cirrus/cirrus_main.c index b119f66fed92..99c1983f99d2 100644 --- a/drivers/gpu/drm/cirrus/cirrus_main.c +++ b/drivers/gpu/drm/cirrus/cirrus_main.c @@ -271,8 +271,6 @@ void cirrus_gem_free_object(struct drm_gem_object *obj) { struct cirrus_bo *cirrus_bo = gem_to_cirrus_bo(obj); - if (!cirrus_bo) - return; cirrus_bo_unref(&cirrus_bo); } From eb649a614d3a2204825cc41f32a7ae79ff74da35 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sat, 5 Apr 2014 10:10:51 +0200 Subject: [PATCH 05/12] drm/ast: Remove unnecessary NULL check in bo_unref ttm_bo_unref unconditionally calls kref_put on it's argument, so the thing can't be NULL without already causing Oopses. Spotted by coverity. Reviewed-by: David Herrmann Reviewed-by: Ian Romanick Signed-off-by: Daniel Vetter --- drivers/gpu/drm/ast/ast_main.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 50535fd5a88d..38941a656312 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -411,10 +411,9 @@ static void ast_bo_unref(struct ast_bo **bo) tbo = &((*bo)->bo); ttm_bo_unref(&tbo); - if (tbo == NULL) - *bo = NULL; - + *bo = NULL; } + void ast_gem_free_object(struct drm_gem_object *obj) { struct ast_bo *ast_bo = gem_to_ast_bo(obj); From 183c1a32cf7175dfaa9e8889a6e9eb330ddc3c75 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sat, 5 Apr 2014 10:11:16 +0200 Subject: [PATCH 06/12] drm/ast: Remove unecessary NULL check in gem_free The ->gem_free_object never gets called with a NULL pointer, the check is redundant. Also checking after the upcast allows compilers to elide it anyway. Spotted by coverity. v2: Fix patch subject. Reviewed-by: David Herrmann Reviewed-by: Ian Romanick Signed-off-by: Daniel Vetter --- drivers/gpu/drm/ast/ast_main.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 38941a656312..01bf9e730acf 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -418,8 +418,6 @@ void ast_gem_free_object(struct drm_gem_object *obj) { struct ast_bo *ast_bo = gem_to_ast_bo(obj); - if (!ast_bo) - return; ast_bo_unref(&ast_bo); } From 2ffd65283bcf120f6bac0d5954c7ccdb845fcbd5 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sat, 5 Apr 2014 10:19:55 +0200 Subject: [PATCH 07/12] drm/via: Remove unecessary NULL check The context_dtor callback is only called once we've successfully loaded the driver, which means dev->dev_private is set up. The check is hence pointless. Also dev->dev_private is deref already above, so compilers are free to elide it anyway. Reviewed-by: David Herrmann Signed-off-by: Daniel Vetter --- drivers/gpu/drm/via/via_mm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/via/via_mm.c b/drivers/gpu/drm/via/via_mm.c index 927889105483..d70b1e1544bf 100644 --- a/drivers/gpu/drm/via/via_mm.c +++ b/drivers/gpu/drm/via/via_mm.c @@ -79,7 +79,7 @@ int via_final_context(struct drm_device *dev, int context) /* Linux specific until context tracking code gets ported to BSD */ /* Last context, perform cleanup */ - if (list_is_singular(&dev->ctxlist) && dev->dev_private) { + if (list_is_singular(&dev->ctxlist)) { DRM_DEBUG("Last Context\n"); drm_irq_uninstall(dev); via_cleanup_futex(dev_priv); From e39a52da87fc49eeb0a91e7db3c30bd61e87bdb7 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sat, 5 Apr 2014 10:25:18 +0200 Subject: [PATCH 08/12] drm/udl: Initialize ret in udl_driver_load We need to set it to -ENODEV when we don't recognize the device. Otherwise we return/print stack garbage. Spotted by coverity. Reviewed-by: David Herrmann Signed-off-by: Daniel Vetter --- drivers/gpu/drm/udl/udl_main.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c index f5ae57406f34..e1038a945f40 100644 --- a/drivers/gpu/drm/udl/udl_main.c +++ b/drivers/gpu/drm/udl/udl_main.c @@ -294,6 +294,7 @@ int udl_driver_load(struct drm_device *dev, unsigned long flags) dev->dev_private = udl; if (!udl_parse_vendor_descriptor(dev, dev->usbdev)) { + ret = -ENODEV; DRM_ERROR("firmware not recognized. Assume incompatible device\n"); goto err; } From dcb1ee57780f2f772015ce9f80b2adc671257228 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sat, 5 Apr 2014 10:43:37 +0200 Subject: [PATCH 09/12] drm/bochs: Remove unnecessary NULL check in bo_unref ttm_bo_unref unconditionally calls kref_put on it's argument, so the thing can't be NULL without already causing Oopses. Spotted by coverity. Reviewed-by: David Herrmann Signed-off-by: Daniel Vetter --- drivers/gpu/drm/bochs/bochs_mm.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/bochs/bochs_mm.c b/drivers/gpu/drm/bochs/bochs_mm.c index f488be55d650..4a239e931aff 100644 --- a/drivers/gpu/drm/bochs/bochs_mm.c +++ b/drivers/gpu/drm/bochs/bochs_mm.c @@ -434,9 +434,7 @@ static void bochs_bo_unref(struct bochs_bo **bo) tbo = &((*bo)->bo); ttm_bo_unref(&tbo); - if (tbo == NULL) - *bo = NULL; - + *bo = NULL; } void bochs_gem_free_object(struct drm_gem_object *obj) From e0c6a73fb191daad6d4ea808a89c6e22ac1b2733 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sat, 5 Apr 2014 10:44:51 +0200 Subject: [PATCH 10/12] drm/bochs: Remove unecessary NULL check in gem_free The ->gem_free_object never gets called with a NULL pointer, the check is redundant. Also checking after the upcast allows compilers to elide it anyway. Noticed while chasing coverity reports, somehow this one here was not flagged. Reviewed-by: David Herrmann Signed-off-by: Daniel Vetter --- drivers/gpu/drm/bochs/bochs_mm.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/bochs/bochs_mm.c b/drivers/gpu/drm/bochs/bochs_mm.c index 4a239e931aff..b9a695d92792 100644 --- a/drivers/gpu/drm/bochs/bochs_mm.c +++ b/drivers/gpu/drm/bochs/bochs_mm.c @@ -441,8 +441,6 @@ void bochs_gem_free_object(struct drm_gem_object *obj) { struct bochs_bo *bochs_bo = gem_to_bochs_bo(obj); - if (!bochs_bo) - return; bochs_bo_unref(&bochs_bo); } From 8268bd48af9aae5e079d3ba8403ae459ff06cbcb Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sat, 5 Apr 2014 18:24:29 +0200 Subject: [PATCH 11/12] drm/i2c/tda998x: Fix signed overflow issue This is C standard hair-splitting, but afaict - sum will be promoted to signed int in computation since uint8_t fits - signed overflow is undefined. No we need to add up an awful lot of bytes to actually make it overflow. But I guess the real risk is gcc spotting this and going bananas. Fix this by simply using unsigned in to force all computations to use the well-defined unsigned behaviour. Spotted by coverity. v2: Simplify the entire computation as suggested by Jean. Cc: Russell King Cc: Rob Clark Cc: Jean-Francois Moine Reviewed-by: David Herrmann Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i2c/tda998x_drv.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 48af5cac1902..240c331405b9 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -568,11 +568,11 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) static uint8_t tda998x_cksum(uint8_t *buf, size_t bytes) { - uint8_t sum = 0; + int sum = 0; while (bytes--) - sum += *buf++; - return (255 - sum) + 1; + sum -= *buf++; + return sum; } #define HB(x) (x) From 10e6856983c21b76d03282b6da86709bdc23eb77 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sat, 5 Apr 2014 11:12:04 +0200 Subject: [PATCH 12/12] drm: Fix error handling in drm_master_create We need to check whether drm_ht_create succeed and clean up if not. Spotted by coverity. Reviewed-by: David Herrmann Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_stub.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index 4c24c3ac1efa..bfa6cb949545 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -128,7 +128,10 @@ struct drm_master *drm_master_create(struct drm_minor *minor) kref_init(&master->refcount); spin_lock_init(&master->lock.spinlock); init_waitqueue_head(&master->lock.lock_queue); - drm_ht_create(&master->magiclist, DRM_MAGIC_HASH_ORDER); + if (drm_ht_create(&master->magiclist, DRM_MAGIC_HASH_ORDER)) { + kfree(master); + return NULL; + } INIT_LIST_HEAD(&master->magicfree); master->minor = minor;