drm/savage: dereferencing an error pointer
diff mbox

Message ID 20161012062227.GU12841@mwanda
State New
Headers show

Commit Message

Dan Carpenter Oct. 12, 2016, 6:22 a.m. UTC
A recent cleanup changed the kmalloc() + copy_from_user() to
memdup_user() but the error handling wasn't updated so we might call
kfree(-EFAULT) and crash.

Fixes: a6e3918bcdb1 ('GPU-DRM-Savage: Use memdup_user() rather than duplicating')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Comments

SF Markus Elfring Oct. 12, 2016, 11:12 a.m. UTC | #1
> A recent cleanup changed the kmalloc() + copy_from_user() to
> memdup_user() but the error handling wasn't updated so we might call
> kfree(-EFAULT) and crash.
> 
> Fixes: a6e3918bcdb1 ('GPU-DRM-Savage: Use memdup_user() rather than duplicating')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/gpu/drm/savage/savage_state.c b/drivers/gpu/drm/savage/savage_state.c
> index 3dc0d8f..2db89be 100644
> --- a/drivers/gpu/drm/savage/savage_state.c
> +++ b/drivers/gpu/drm/savage/savage_state.c
> @@ -1004,6 +1004,7 @@ int savage_bci_cmdbuf(struct drm_device *dev, void *data, struct drm_file *file_
>  		kvb_addr = memdup_user(cmdbuf->vb_addr, cmdbuf->vb_size);
>  		if (IS_ERR(kvb_addr)) {
>  			ret = PTR_ERR(kvb_addr);
> +			kvb_addr = NULL;
>  			goto done;
>  		}
>  		cmdbuf->vb_addr = kvb_addr;
> 

Thanks for this update suggestion.

Can it be that I offered an other approach for a corresponding software correction
by the update step “[PATCH 2/2] GPU-DRM-Savage: Less function calls in
savage_bci_cmdbuf() after error detection” (on 2016-08-18)?

https://patchwork.kernel.org/patch/9289183/
https://lkml.kernel.org/r/<c97563c0-d463-8b15-5956-26d93641a54f@users.sourceforge.net>

Will this one become worth for further development consideratons once more?

Can the shown resetting of an error pointer to a safe null pointer be omitted
in such use cases when the jump targets will be accordingly configured as it is
desired for efficient exception handling implementations?

Regards,
Markus

Patch
diff mbox

diff --git a/drivers/gpu/drm/savage/savage_state.c b/drivers/gpu/drm/savage/savage_state.c
index 3dc0d8f..2db89be 100644
--- a/drivers/gpu/drm/savage/savage_state.c
+++ b/drivers/gpu/drm/savage/savage_state.c
@@ -1004,6 +1004,7 @@  int savage_bci_cmdbuf(struct drm_device *dev, void *data, struct drm_file *file_
 		kvb_addr = memdup_user(cmdbuf->vb_addr, cmdbuf->vb_size);
 		if (IS_ERR(kvb_addr)) {
 			ret = PTR_ERR(kvb_addr);
+			kvb_addr = NULL;
 			goto done;
 		}
 		cmdbuf->vb_addr = kvb_addr;