* [PATCH][media-next][V2] media: davinci_vpfe: fix memory leaks of params
@ 2018-05-02 11:48 ` Colin King
0 siblings, 0 replies; 4+ messages in thread
From: Colin King @ 2018-05-02 11:48 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, devel
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There are memory leaks of params; when copy_to_user fails and also
the exit via the label 'error'. Also, there is a bogos memory allocation
check on pointer 'to' when memory allocation fails on params.
Fix this by kfree'ing params in error exit path and jumping to this on
the copy_to_user failure path. Also check the to see if the allocation
of params fails and remove the bogus null pointer checks on pointer 'to'.
Also explicitly return 0 on success rather than rval.
Detected by CoverityScan, CID#1467966 ("Resource leak")
Fixes: da43b6ccadcf ("[media] davinci: vpfe: dm365: add IPIPE support for media controller driver")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
V2: Add checks on allocation of params. Remove bogus checks on
pointer 'to'. Explicitly return 0 on success. Thanks to
Dan Carpenter for the suggested improvements.
---
drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
index 95942768639c..b135e38a18b3 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
@@ -1252,12 +1252,12 @@ static const struct ipipe_module_if ipipe_modules[VPFE_IPIPE_MAX_MODULES] = {
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;
for (i = 0; i < ARRAY_SIZE(ipipe_modules); i++) {
const struct ipipe_module_if *module_if;
- struct ipipe_module_params *params;
void *from, *to;
size_t size;
@@ -1269,25 +1269,31 @@ 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) {
+ rval = -ENOMEM;
+ goto error;
+ }
to = (void *)params + module_if->param_offset;
size = module_if->param_size;
- if (to && from && size) {
+ if (from && size) {
if (copy_from_user(to, (void __user *)from, size)) {
rval = -EFAULT;
- break;
+ goto error;
}
rval = module_if->set(ipipe, to);
if (rval)
goto error;
- } else if (to && !from && size) {
+ } else if (!from && size) {
rval = module_if->set(ipipe, NULL);
if (rval)
goto error;
}
kfree(params);
}
+ return 0;
error:
+ kfree(params);
return rval;
}
--
2.17.0
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH][media-next][V2] media: davinci_vpfe: fix memory leaks of params
@ 2018-05-02 11:48 ` Colin King
0 siblings, 0 replies; 4+ messages in thread
From: Colin King @ 2018-05-02 11:48 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, devel
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There are memory leaks of params; when copy_to_user fails and also
the exit via the label 'error'. Also, there is a bogos memory allocation
check on pointer 'to' when memory allocation fails on params.
Fix this by kfree'ing params in error exit path and jumping to this on
the copy_to_user failure path. Also check the to see if the allocation
of params fails and remove the bogus null pointer checks on pointer 'to'.
Also explicitly return 0 on success rather than rval.
Detected by CoverityScan, CID#1467966 ("Resource leak")
Fixes: da43b6ccadcf ("[media] davinci: vpfe: dm365: add IPIPE support for media controller driver")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
V2: Add checks on allocation of params. Remove bogus checks on
pointer 'to'. Explicitly return 0 on success. Thanks to
Dan Carpenter for the suggested improvements.
---
drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
index 95942768639c..b135e38a18b3 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
@@ -1252,12 +1252,12 @@ static const struct ipipe_module_if ipipe_modules[VPFE_IPIPE_MAX_MODULES] = {
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;
for (i = 0; i < ARRAY_SIZE(ipipe_modules); i++) {
const struct ipipe_module_if *module_if;
- struct ipipe_module_params *params;
void *from, *to;
size_t size;
@@ -1269,25 +1269,31 @@ 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) {
+ rval = -ENOMEM;
+ goto error;
+ }
to = (void *)params + module_if->param_offset;
size = module_if->param_size;
- if (to && from && size) {
+ if (from && size) {
if (copy_from_user(to, (void __user *)from, size)) {
rval = -EFAULT;
- break;
+ goto error;
}
rval = module_if->set(ipipe, to);
if (rval)
goto error;
- } else if (to && !from && size) {
+ } else if (!from && size) {
rval = module_if->set(ipipe, NULL);
if (rval)
goto error;
}
kfree(params);
}
+ return 0;
error:
+ kfree(params);
return rval;
}
--
2.17.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH][media-next][V2] media: davinci_vpfe: fix memory leaks of params
@ 2018-05-02 11:48 ` Colin King
0 siblings, 0 replies; 4+ messages in thread
From: Colin King @ 2018-05-02 11:48 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, devel
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There are memory leaks of params; when copy_to_user fails and also
the exit via the label 'error'. Also, there is a bogos memory allocation
check on pointer 'to' when memory allocation fails on params.
Fix this by kfree'ing params in error exit path and jumping to this on
the copy_to_user failure path. Also check the to see if the allocation
of params fails and remove the bogus null pointer checks on pointer 'to'.
Also explicitly return 0 on success rather than rval.
Detected by CoverityScan, CID#1467966 ("Resource leak")
Fixes: da43b6ccadcf ("[media] davinci: vpfe: dm365: add IPIPE support for media controller driver")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
V2: Add checks on allocation of params. Remove bogus checks on
pointer 'to'. Explicitly return 0 on success. Thanks to
Dan Carpenter for the suggested improvements.
---
drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
index 95942768639c..b135e38a18b3 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
@@ -1252,12 +1252,12 @@ static const struct ipipe_module_if ipipe_modules[VPFE_IPIPE_MAX_MODULES] = {
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;
for (i = 0; i < ARRAY_SIZE(ipipe_modules); i++) {
const struct ipipe_module_if *module_if;
- struct ipipe_module_params *params;
void *from, *to;
size_t size;
@@ -1269,25 +1269,31 @@ 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) {
+ rval = -ENOMEM;
+ goto error;
+ }
to = (void *)params + module_if->param_offset;
size = module_if->param_size;
- if (to && from && size) {
+ if (from && size) {
if (copy_from_user(to, (void __user *)from, size)) {
rval = -EFAULT;
- break;
+ goto error;
}
rval = module_if->set(ipipe, to);
if (rval)
goto error;
- } else if (to && !from && size) {
+ } else if (!from && size) {
rval = module_if->set(ipipe, NULL);
if (rval)
goto error;
}
kfree(params);
}
+ return 0;
error:
+ kfree(params);
return rval;
}
--
2.17.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH][media-next][V2] media: davinci_vpfe: fix memory leaks of params
2018-05-02 11:48 ` Colin King
(?)
(?)
@ 2018-05-02 12:23 ` Walter Harms
-1 siblings, 0 replies; 4+ messages in thread
From: Walter Harms @ 2018-05-02 12:23 UTC (permalink / raw)
To: kernel-janitors
Am 02.05.2018 13:48, schrieb Colin King:
> From: Colin Ian King <colin.king@canonical.com>
>
> There are memory leaks of params; when copy_to_user fails and also
> the exit via the label 'error'. Also, there is a bogos memory allocation
> check on pointer 'to' when memory allocation fails on params.
>
> Fix this by kfree'ing params in error exit path and jumping to this on
> the copy_to_user failure path. Also check the to see if the allocation
> of params fails and remove the bogus null pointer checks on pointer 'to'.
>
> Also explicitly return 0 on success rather than rval.
>
> Detected by CoverityScan, CID#1467966 ("Resource leak")
>
> Fixes: da43b6ccadcf ("[media] davinci: vpfe: dm365: add IPIPE support for
> media controller driver")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>
> V2: Add checks on allocation of params. Remove bogus checks on
> pointer 'to'. Explicitly return 0 on success. Thanks to
> Dan Carpenter for the suggested improvements.
>
Hi Colin,
the code made me thinking a bit as it seems bit complicated.
I did not dive into the full code only the patch so i may have
some false assumptions.
> ---
> drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> index 95942768639c..b135e38a18b3 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> @@ -1252,12 +1252,12 @@ static const struct ipipe_module_if
> ipipe_modules[VPFE_IPIPE_MAX_MODULES] = {
> 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;
>
> for (i = 0; i < ARRAY_SIZE(ipipe_modules); i++) {
> const struct ipipe_module_if *module_if;
> - struct ipipe_module_params *params;
> void *from, *to;
> size_t size;
>
> @@ -1269,25 +1269,31 @@ 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) {
> + rval = -ENOMEM;
> + goto error;
> + }
A single exit is nice but maybe a
return -ENOMEM;
is ok here.
> to = (void *)params + module_if->param_offset;
> size = module_if->param_size;
>
the if (from) make no sense if size = 0
if (size = 0) {
rval = 0;
goto error;
}
the following code reduces to
if (from)
....
else
....
sorry for beeing late,
re,
wh
> - if (to && from && size) {
> + if (from && size) {
> if (copy_from_user(to, (void __user *)from, size)) {
> rval = -EFAULT;
> - break;
> + goto error;
> }
> rval = module_if->set(ipipe, to);
> if (rval)
> goto error;
> - } else if (to && !from && size) {
> + } else if (!from && size) {
> rval = module_if->set(ipipe, NULL);
> if (rval)
> goto error;
> }
> kfree(params);
> }
> + return 0;
> error:
> + kfree(params);
> return rval;
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-05-02 12:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-02 11:48 [PATCH][media-next][V2] media: davinci_vpfe: fix memory leaks of params Colin King
2018-05-02 11:48 ` Colin King
2018-05-02 11:48 ` Colin King
2018-05-02 12:23 ` Walter Harms
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.