[PATCH v3 1/2] staging: media: davinci_vpfe: add error handling on kmalloc failure

[PATCH v3 1/2] staging: media: davinci_vpfe: add error handling on kmalloc failure

[Ji-Hun Kim]

There is no failure checking on the param value which will be allocated
memory by kmalloc. Add a null pointer checking statement. Then goto error:
and return -ENOMEM error code when kmalloc is failed.

Signed-off-by: Ji-Hun Kim <ji_hun.kim@samsung.com>
---
Changes since v1:
  - Return with -ENOMEM directly, instead of goto error: then return.
  - [Patch v3 1/2] is same with [patch v2]

 drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
index 6a3434c..ffcd86d 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
@@ -1280,6 +1280,9 @@ static int ipipe_s_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg)
 
 			params = kmalloc(sizeof(struct ipipe_module_params),
 					 GFP_KERNEL);
+			if (!params)
+				return -ENOMEM;
+
 			to = (void *)params + module_if->param_offset;
 			size = module_if->param_size;
 
@@ -1323,6 +1326,9 @@ static int ipipe_g_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg)
 
 			params =  kmalloc(sizeof(struct ipipe_module_params),
 						GFP_KERNEL);
+			if (!params)
+				return -ENOMEM;
+
 			from = (void *)params + module_if->param_offset;
 			size = module_if->param_size;
 
-- 
1.9.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 2/2] staging: media: davinci_vpfe: add kfree() on goto err statement

[Ji-Hun Kim]

It needs to free of allocated params value in the goto error statement.

Signed-off-by: Ji-Hun Kim <ji_hun.kim@samsung.com>
---
Changes since v2:
  - add kfree(params) on the error case of the function
  - rename unclear goto statement name
  - declare the params value at start of the function, so it can be free
    end of the function

 drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
index ffcd86d..735d8b5 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
@@ -1263,6 +1263,7 @@ static int ipipe_get_cgs_params(struct vpfe_ipipe_device *ipipe, void *param)
 static int ipipe_s_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg)
 {
 	struct vpfe_ipipe_device *ipipe = v4l2_get_subdevdata(sd);
+	struct ipipe_module_params *params;
 	unsigned int i;
 	int rval = 0;
 
@@ -1272,7 +1273,6 @@ static int ipipe_s_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg)
 		if (cfg->flag & bit) {
 			const struct ipipe_module_if *module_if =
 						&ipipe_modules[i];
-			struct ipipe_module_params *params;
 			void __user *from = *(void * __user *)
 				((void *)cfg + module_if->config_offset);
 			size_t size;
@@ -1289,26 +1289,30 @@ static int ipipe_s_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg)
 			if (to && from && size) {
 				if (copy_from_user(to, from, size)) {
 					rval = -EFAULT;
-					break;
+					goto err_free_params;
 				}
 				rval = module_if->set(ipipe, to);
 				if (rval)
-					goto error;
+					goto err_free_params;
 			} else if (to && !from && size) {
 				rval = module_if->set(ipipe, NULL);
 				if (rval)
-					goto error;
+					goto err_free_params;
 			}
 			kfree(params);
 		}
 	}
-error:
+	return 0;
+
+err_free_params:
+	kfree(params);
 	return rval;
 }
 
 static int ipipe_g_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg)
 {
 	struct vpfe_ipipe_device *ipipe = v4l2_get_subdevdata(sd);
+	struct ipipe_module_params *params;
 	unsigned int i;
 	int rval = 0;
 
@@ -1318,7 +1322,6 @@ static int ipipe_g_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg)
 		if (cfg->flag & bit) {
 			const struct ipipe_module_if *module_if =
 						&ipipe_modules[i];
-			struct ipipe_module_params *params;
 			void __user *to = *(void * __user *)
 				((void *)cfg + module_if->config_offset);
 			size_t size;
@@ -1335,16 +1338,19 @@ static int ipipe_g_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg)
 			if (to && from && size) {
 				rval = module_if->get(ipipe, from);
 				if (rval)
-					goto error;
+					goto err_free_params;
 				if (copy_to_user(to, from, size)) {
 					rval = -EFAULT;
-					break;
+					goto err_free_params;
 				}
 			}
 			kfree(params);
 		}
 	}
-error:
+	return 0;
+
+err_free_params:
+	kfree(params);
 	return rval;
 }
 
-- 
1.9.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 2/2] staging: media: davinci_vpfe: add kfree() on goto err statement

[Dan Carpenter]

Looks good.  Thanks!

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 1/2] staging: media: davinci_vpfe: add error handling on kmalloc failure

[Ji-Hun Kim]

On Wed, Mar 21, 2018 at 01:39:09PM +0900, Ji-Hun Kim wrote:
> There is no failure checking on the param value which will be allocated
> memory by kmalloc. Add a null pointer checking statement. Then goto error:
> and return -ENOMEM error code when kmalloc is failed.
> 
> Signed-off-by: Ji-Hun Kim <ji_hun.kim@samsung.com>
> ---
> Changes since v1:
>   - Return with -ENOMEM directly, instead of goto error: then return.
>   - [Patch v3 1/2] is same with [patch v2]
> 
>  drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> index 6a3434c..ffcd86d 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> @@ -1280,6 +1280,9 @@ static int ipipe_s_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg)
>  
>  			params = kmalloc(sizeof(struct ipipe_module_params),
>  					 GFP_KERNEL);
> +			if (!params)
> +				return -ENOMEM;
> +
>  			to = (void *)params + module_if->param_offset;
>  			size = module_if->param_size;
>  
> @@ -1323,6 +1326,9 @@ static int ipipe_g_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg)
>  
>  			params =  kmalloc(sizeof(struct ipipe_module_params),
>  						GFP_KERNEL);
> +			if (!params)
> +				return -ENOMEM;
> +
>  			from = (void *)params + module_if->param_offset;
>  			size = module_if->param_size;
>  
> -- 
> 1.9.1
> 
> 

Are there any opinions? I'd like to know how this patch is going.

Best regards,
Ji-Hun

Re: [PATCH v3 1/2] staging: media: davinci_vpfe: add error handling on kmalloc failure

[Dan Carpenter]

On Tue, Mar 27, 2018 at 02:00:45PM +0900, Ji-Hun Kim wrote:
> 
> Are there any opinions? I'd like to know how this patch is going.
> 


Looks good.  Thanks!

Greg just hasn't gotten to it yet.

regards,
dan carpenter


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 1/2] staging: media: davinci_vpfe: add error handling on kmalloc failure

[Greg KH]

On Tue, Mar 27, 2018 at 08:20:59AM +0300, Dan Carpenter wrote:
> On Tue, Mar 27, 2018 at 02:00:45PM +0900, Ji-Hun Kim wrote:
> > 
> > Are there any opinions? I'd like to know how this patch is going.
> > 
> 
> 
> Looks good.  Thanks!
> 
> Greg just hasn't gotten to it yet.

Greg does not take drivers/staging/media/* patches :)
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel