* [PATCH] staging: vchiq: Fix list_for_each exit tests
@ 2020-09-28 9:11 Dan Carpenter
2020-10-01 8:56 ` Greg Kroah-Hartman
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2020-09-28 9:11 UTC (permalink / raw)
To: Nicolas Saenz Julienne
Cc: Stefan Wahren, devel, Arnd Bergmann, Greg Kroah-Hartman,
Marcelo Diop-Gonzalez, kernel-janitors, bcm-kernel-feedback-list,
linux-rpi-kernel, Jamal Shareef
This code code here used to be list_for_each() and after the loop then
the "entry" pointer was non-NULL if we found the correct entry or NULL
if we couldn't find it. Then we changed the code to use list_for_each_entry()
and so now the "entry" pointer is always non-NULL when we exit the loop.
I have introduced a new "found" variable to say if we found the correct
enty or not. I fixed one test by making it an else statement because
that was more readable than testing "if (!found)".
Fixes: 46e4b9ec4fa4 ("staging: vchiq_arm: use list_for_each_entry when accessing bulk_waiter_list")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
.../vc04_services/interface/vchiq_arm/vchiq_arm.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index bb0cc9cb96e9..c25fc559cd36 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -430,6 +430,7 @@ vchiq_blocking_bulk_transfer(unsigned int handle, void *data,
struct vchiq_service *service;
enum vchiq_status status;
struct bulk_waiter_node *waiter = NULL;
+ bool found = false;
service = find_service_by_handle(handle);
if (!service)
@@ -443,12 +444,13 @@ vchiq_blocking_bulk_transfer(unsigned int handle, void *data,
list_for_each_entry(waiter, &instance->bulk_waiter_list, list) {
if (waiter->pid == current->pid) {
list_del(&waiter->list);
+ found = true;
break;
}
}
mutex_unlock(&instance->bulk_waiter_list_mutex);
- if (waiter) {
+ if (found) {
struct vchiq_bulk *bulk = waiter->bulk_waiter.bulk;
if (bulk) {
@@ -464,9 +466,7 @@ vchiq_blocking_bulk_transfer(unsigned int handle, void *data,
spin_unlock(&bulk_waiter_spinlock);
}
}
- }
-
- if (!waiter) {
+ } else {
waiter = kzalloc(sizeof(struct bulk_waiter_node), GFP_KERNEL);
if (!waiter) {
vchiq_log_error(vchiq_core_log_level,
@@ -945,6 +945,7 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
{
struct vchiq_service *service;
struct bulk_waiter_node *waiter = NULL;
+ bool found = false;
int status = 0;
int ret;
@@ -967,11 +968,12 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
list) {
if (waiter->pid == current->pid) {
list_del(&waiter->list);
+ found = true;
break;
}
}
mutex_unlock(&instance->bulk_waiter_list_mutex);
- if (!waiter) {
+ if (!found) {
vchiq_log_error(vchiq_arm_log_level,
"no bulk_waiter found for pid %d",
current->pid);
--
2.28.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
* Re: [PATCH] staging: vchiq: Fix list_for_each exit tests
2020-09-28 9:11 [PATCH] staging: vchiq: Fix list_for_each exit tests Dan Carpenter
@ 2020-10-01 8:56 ` Greg Kroah-Hartman
2020-10-06 13:47 ` [PATCH v2] " Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Greg Kroah-Hartman @ 2020-10-01 8:56 UTC (permalink / raw)
To: Dan Carpenter
Cc: Stefan Wahren, devel, Arnd Bergmann, Marcelo Diop-Gonzalez,
kernel-janitors, bcm-kernel-feedback-list, linux-rpi-kernel,
Jamal Shareef, Nicolas Saenz Julienne
On Mon, Sep 28, 2020 at 12:11:03PM +0300, Dan Carpenter wrote:
> This code code here used to be list_for_each() and after the loop then
> the "entry" pointer was non-NULL if we found the correct entry or NULL
> if we couldn't find it. Then we changed the code to use list_for_each_entry()
> and so now the "entry" pointer is always non-NULL when we exit the loop.
>
> I have introduced a new "found" variable to say if we found the correct
> enty or not. I fixed one test by making it an else statement because
> that was more readable than testing "if (!found)".
>
> Fixes: 46e4b9ec4fa4 ("staging: vchiq_arm: use list_for_each_entry when accessing bulk_waiter_list")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
This doesn't apply against my tree, what branch did you make it against?
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] staging: vchiq: Fix list_for_each exit tests
2020-10-01 8:56 ` Greg Kroah-Hartman
@ 2020-10-06 13:47 ` Dan Carpenter
2020-10-08 9:38 ` Nicolas Saenz Julienne
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2020-10-06 13:47 UTC (permalink / raw)
To: Nicolas Saenz Julienne
Cc: Stefan Wahren, devel, Arnd Bergmann, Greg Kroah-Hartman,
Marcelo Diop-Gonzalez, kernel-janitors, bcm-kernel-feedback-list,
linux-rpi-kernel, Jamal Shareef
After a list_for_each_entry() loop, the list iterator is always non-NULL
so these conditions don't work. If the "waiter" is not found then this
results in an out of bounds access.
I have fixed it by introducing a new "found" variable. In one case, I
used an else statement for readability.
Fixes: 46e4b9ec4fa4 ("staging: vchiq_arm: use list_for_each_entry when accessing bulk_waiter_list")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: rebase on latest linux-next
.../vc04_services/interface/vchiq_arm/vchiq_arm.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 590415561b73..e6a9aab66f4a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -432,6 +432,7 @@ vchiq_blocking_bulk_transfer(unsigned int handle, void *data,
struct vchiq_service *service;
enum vchiq_status status;
struct bulk_waiter_node *waiter = NULL;
+ bool found = false;
service = find_service_by_handle(handle);
if (!service)
@@ -445,12 +446,13 @@ vchiq_blocking_bulk_transfer(unsigned int handle, void *data,
list_for_each_entry(waiter, &instance->bulk_waiter_list, list) {
if (waiter->pid == current->pid) {
list_del(&waiter->list);
+ found = true;
break;
}
}
mutex_unlock(&instance->bulk_waiter_list_mutex);
- if (waiter) {
+ if (found) {
struct vchiq_bulk *bulk = waiter->bulk_waiter.bulk;
if (bulk) {
@@ -467,9 +469,7 @@ vchiq_blocking_bulk_transfer(unsigned int handle, void *data,
spin_unlock(&bulk_waiter_spinlock);
}
}
- }
-
- if (!waiter) {
+ } else {
waiter = kzalloc(sizeof(struct bulk_waiter_node), GFP_KERNEL);
if (!waiter) {
vchiq_log_error(vchiq_core_log_level,
@@ -952,6 +952,7 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
{
struct vchiq_service *service;
struct bulk_waiter_node *waiter = NULL;
+ bool found = false;
void *userdata;
int status = 0;
int ret;
@@ -975,11 +976,12 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
list) {
if (waiter->pid == current->pid) {
list_del(&waiter->list);
+ found = true;
break;
}
}
mutex_unlock(&instance->bulk_waiter_list_mutex);
- if (!waiter) {
+ if (!found) {
vchiq_log_error(vchiq_arm_log_level,
"no bulk_waiter found for pid %d",
current->pid);
--
2.28.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
* Re: [PATCH v2] staging: vchiq: Fix list_for_each exit tests
2020-10-06 13:47 ` [PATCH v2] " Dan Carpenter
@ 2020-10-08 9:38 ` Nicolas Saenz Julienne
0 siblings, 0 replies; 4+ messages in thread
From: Nicolas Saenz Julienne @ 2020-10-08 9:38 UTC (permalink / raw)
To: Dan Carpenter
Cc: Stefan Wahren, devel, Arnd Bergmann, Greg Kroah-Hartman,
Marcelo Diop-Gonzalez, kernel-janitors, bcm-kernel-feedback-list,
linux-rpi-kernel, Jamal Shareef
[-- Attachment #1.1: Type: text/plain, Size: 607 bytes --]
On Tue, 2020-10-06 at 16:47 +0300, Dan Carpenter wrote:
> After a list_for_each_entry() loop, the list iterator is always non-NULL
> so these conditions don't work. If the "waiter" is not found then this
> results in an out of bounds access.
>
> I have fixed it by introducing a new "found" variable. In one case, I
> used an else statement for readability.
>
> Fixes: 46e4b9ec4fa4 ("staging: vchiq_arm: use list_for_each_entry when accessing bulk_waiter_list")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 169 bytes --]
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-10-08 9:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-28 9:11 [PATCH] staging: vchiq: Fix list_for_each exit tests Dan Carpenter
2020-10-01 8:56 ` Greg Kroah-Hartman
2020-10-06 13:47 ` [PATCH v2] " Dan Carpenter
2020-10-08 9:38 ` Nicolas Saenz Julienne
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).