diff mbox series

staging: vc04_services: Fix RCU dereference check

Message ID 20211229153243.6064-1-treasure4paddy@gmail.com (mailing list archive)
State New, archived
Headers show
Series staging: vc04_services: Fix RCU dereference check | expand

Commit Message

Padmanabha Srinivasaiah Dec. 29, 2021, 3:32 p.m. UTC 
In service_callback path RCU dereference done without
rcu_read_[lock/unlock] pair, fixing same by using them.

[   32.201659] =============================
[   32.201664] WARNING: suspicious RCU usage
[   32.201670] 5.15.11-rt24-v8+ #3 Not tainted
[   32.201680] -----------------------------
[   32.201685] drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h:529 suspicious rcu_dereference_check() usage!
[   32.201695]
[   32.201695] other info that might help us debug this:
[   32.201695]
[   32.201700]
[   32.201700] rcu_scheduler_active = 2, debug_locks = 1
[   32.201708] no locks held by vchiq-slot/0/98.
[   32.201715]
[   32.201715] stack backtrace:
[   32.201723] CPU: 1 PID: 98 Comm: vchiq-slot/0 Not tainted 5.15.11-rt24-v8+ #3
[   32.201733] Hardware name: Raspberry Pi 4 Model B Rev 1.4 (DT)
[   32.201739] Call trace:
[   32.201742]  dump_backtrace+0x0/0x1b8
[   32.201772]  show_stack+0x20/0x30
[   32.201784]  dump_stack_lvl+0x8c/0xb8
[   32.201799]  dump_stack+0x18/0x34
[   32.201808]  lockdep_rcu_suspicious+0xe4/0xf8
[   32.201817]  service_callback+0x124/0x400
[   32.201830]  slot_handler_func+0xf60/0x1e20
[   32.201839]  kthread+0x19c/0x1a8
[   32.201849]  ret_from_fork+0x10/0x20

Signed-off-by: Padmanabha Srinivasaiah <treasure4paddy@gmail.com>
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Stefan Wahren Dec. 29, 2021, 7:13 p.m. UTC | #1 
Hi Padmanabha,

Am 29.12.21 um 16:32 schrieb Padmanabha Srinivasaiah:
> In service_callback path RCU dereference done without
> rcu_read_[lock/unlock] pair, fixing same by using them.
>
> [   32.201659] =============================
> [   32.201664] WARNING: suspicious RCU usage
> [   32.201670] 5.15.11-rt24-v8+ #3 Not tainted
> [   32.201680] -----------------------------
> [   32.201685] drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h:529 suspicious rcu_dereference_check() usage!
> [   32.201695]
> [   32.201695] other info that might help us debug this:
> [   32.201695]
> [   32.201700]
> [   32.201700] rcu_scheduler_active = 2, debug_locks = 1
> [   32.201708] no locks held by vchiq-slot/0/98.
> [   32.201715]
> [   32.201715] stack backtrace:
> [   32.201723] CPU: 1 PID: 98 Comm: vchiq-slot/0 Not tainted 5.15.11-rt24-v8+ #3
> [   32.201733] Hardware name: Raspberry Pi 4 Model B Rev 1.4 (DT)
> [   32.201739] Call trace:
> [   32.201742]  dump_backtrace+0x0/0x1b8
> [   32.201772]  show_stack+0x20/0x30
> [   32.201784]  dump_stack_lvl+0x8c/0xb8
> [   32.201799]  dump_stack+0x18/0x34
> [   32.201808]  lockdep_rcu_suspicious+0xe4/0xf8
> [   32.201817]  service_callback+0x124/0x400
> [   32.201830]  slot_handler_func+0xf60/0x1e20
> [   32.201839]  kthread+0x19c/0x1a8
> [   32.201849]  ret_from_fork+0x10/0x20
>
> Signed-off-by: Padmanabha Srinivasaiah <treasure4paddy@gmail.com>
> ---
>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> 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 6759a6261500..ee1b48db9681 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -1058,7 +1058,10 @@ service_callback(enum vchiq_reason reason, struct vchiq_header *header,
>  
>  	DEBUG_TRACE(SERVICE_CALLBACK_LINE);
>  
> +	rcu_read_lock();
>  	service = handle_to_service(handle);
> +	rcu_read_unlock();
> +
>  	if (WARN_ON(!service))
>  		return VCHIQ_SUCCESS;
>  

thanks for reporting this issue.

Could you please explain how to reproduce this issue?

What makes you sure that your patch fixes the issue and not just hiding
the warning?

Best regards
Padmanabha Srinivasaiah Dec. 30, 2021, 1:34 p.m. UTC | #2 
On Wed, Dec 29, 2021 at 08:13:40PM +0100, Stefan Wahren wrote:
> Hi Padmanabha,
> 
> Am 29.12.21 um 16:32 schrieb Padmanabha Srinivasaiah:
> > In service_callback path RCU dereference done without
> > rcu_read_[lock/unlock] pair, fixing same by using them.
> >
> > [   32.201659] =============================
> > [   32.201664] WARNING: suspicious RCU usage
> > [   32.201670] 5.15.11-rt24-v8+ #3 Not tainted
> > [   32.201680] -----------------------------
> > [   32.201685] drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h:529 suspicious rcu_dereference_check() usage!
> > [   32.201695]
> > [   32.201695] other info that might help us debug this:
> > [   32.201695]
> > [   32.201700]
> > [   32.201700] rcu_scheduler_active = 2, debug_locks = 1
> > [   32.201708] no locks held by vchiq-slot/0/98.
> > [   32.201715]
> > [   32.201715] stack backtrace:
> > [   32.201723] CPU: 1 PID: 98 Comm: vchiq-slot/0 Not tainted 5.15.11-rt24-v8+ #3
> > [   32.201733] Hardware name: Raspberry Pi 4 Model B Rev 1.4 (DT)
> > [   32.201739] Call trace:
> > [   32.201742]  dump_backtrace+0x0/0x1b8
> > [   32.201772]  show_stack+0x20/0x30
> > [   32.201784]  dump_stack_lvl+0x8c/0xb8
> > [   32.201799]  dump_stack+0x18/0x34
> > [   32.201808]  lockdep_rcu_suspicious+0xe4/0xf8
> > [   32.201817]  service_callback+0x124/0x400
> > [   32.201830]  slot_handler_func+0xf60/0x1e20
> > [   32.201839]  kthread+0x19c/0x1a8
> > [   32.201849]  ret_from_fork+0x10/0x20
> >
> > Signed-off-by: Padmanabha Srinivasaiah <treasure4paddy@gmail.com>
> > ---
> >  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > 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 6759a6261500..ee1b48db9681 100644
> > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > @@ -1058,7 +1058,10 @@ service_callback(enum vchiq_reason reason, struct vchiq_header *header,
> >  
> >  	DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> >  
> > +	rcu_read_lock();
> >  	service = handle_to_service(handle);
> > +	rcu_read_unlock();
> > +
> >  	if (WARN_ON(!service))
> >  		return VCHIQ_SUCCESS;
> >  
> 
> thanks for reporting this issue.
> 
> Could you please explain how to reproduce this issue?

Have enabled lock dependency check in 5.15 plus rt patchset have been
applied. And to catch this warning not loading any modules from default
build.

> 
> What makes you sure that your patch fixes the issue and not just hiding
> the warning?

Thank you stefan for the question. Yes, you are right dereferenced pointer
access should be inside the rcu read-side critical section. Will regenerate the patch
and will share for the review.

> Best regards
>
diff mbox series

Patch

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 6759a6261500..ee1b48db9681 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1058,7 +1058,10 @@  service_callback(enum vchiq_reason reason, struct vchiq_header *header,
 
 	DEBUG_TRACE(SERVICE_CALLBACK_LINE);
 
+	rcu_read_lock();
 	service = handle_to_service(handle);
+	rcu_read_unlock();
+
 	if (WARN_ON(!service))
 		return VCHIQ_SUCCESS;