diff mbox

mailbox: mailbox-test: set tdev->signal to NULL after freeing

Message ID 1464106324-25005-1-git-send-email-sudeep.holla@arm.com
State Accepted
Commit 9ef3c5112139cc5c5666ee096e05bc1e00e94015
Headers show

Commit Message

Sudeep Holla May 24, 2016, 4:12 p.m. UTC
tdev->signal is not set NULL after it's freed. This will cause random
exceptions when the stale pointer is accessed after tdev->signal is
freed. Also, since tdev->signal allocation is skipped the next time
it's written, this leads to continuous fault finally leading to the
total death of the system.

Cc: Lee Jones <lee.jones@linaro.org>
Cc: Jassi Brar <jassisinghbrar@gmail.com>
Fixes: d1c2f87c9a8f ("mailbox: mailbox-test: Prevent memory leak")
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

---
 drivers/mailbox/mailbox-test.c | 1 +
 1 file changed, 1 insertion(+)

--
1.9.1

Comments

Sudeep Holla June 6, 2016, 9:39 a.m. UTC | #1
Hi Jassi, Lee,

On 24/05/16 17:12, Sudeep Holla wrote:
> tdev->signal is not set NULL after it's freed. This will cause random

> exceptions when the stale pointer is accessed after tdev->signal is

> freed. Also, since tdev->signal allocation is skipped the next time

> it's written, this leads to continuous fault finally leading to the

> total death of the system.

>

> Cc: Lee Jones <lee.jones@linaro.org>

> Cc: Jassi Brar <jassisinghbrar@gmail.com>


Can you review this ?

-- 
Regards,
Sudeep
Lee Jones June 6, 2016, 3:15 p.m. UTC | #2
On Tue, 24 May 2016, Sudeep Holla wrote:

> tdev->signal is not set NULL after it's freed. This will cause random

> exceptions when the stale pointer is accessed after tdev->signal is

> freed. Also, since tdev->signal allocation is skipped the next time

> it's written, this leads to continuous fault finally leading to the

> total death of the system.

> 

> Cc: Lee Jones <lee.jones@linaro.org>

> Cc: Jassi Brar <jassisinghbrar@gmail.com>

> Fixes: d1c2f87c9a8f ("mailbox: mailbox-test: Prevent memory leak")

> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

> ---

>  drivers/mailbox/mailbox-test.c | 1 +

>  1 file changed, 1 insertion(+)


Strange there isn't a kfree_and_null() call.

Patch looks okay to me though:

Acked-by: Lee Jones <lee.jones@linaro.org>


> diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c

> index 58d04726cdd7..9ca96e9db6bf 100644

> --- a/drivers/mailbox/mailbox-test.c

> +++ b/drivers/mailbox/mailbox-test.c

> @@ -133,6 +133,7 @@ static ssize_t mbox_test_message_write(struct file *filp,

>  out:

>  	kfree(tdev->signal);

>  	kfree(tdev->message);

> +	tdev->signal = NULL;

> 

>  	return ret < 0 ? ret : count;

>  }


-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
diff mbox

Patch

diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c
index 58d04726cdd7..9ca96e9db6bf 100644
--- a/drivers/mailbox/mailbox-test.c
+++ b/drivers/mailbox/mailbox-test.c
@@ -133,6 +133,7 @@  static ssize_t mbox_test_message_write(struct file *filp,
 out:
 	kfree(tdev->signal);
 	kfree(tdev->message);
+	tdev->signal = NULL;

 	return ret < 0 ? ret : count;
 }