On Tue, Jun 14, 2022 at 02:25:44PM -0400, Adrien Thierry wrote: > In service_callback(), the vchiq_instance is checked for NULL after > dereferencing it. Switch the order of those operations. > > This was reported by https://lore.kernel.org/all/Yqc+Oavmh0zMRVPQ@kili/ > > I moved the NULL check before the call to rcu_read_lock() since access > to the vchiq_instance is not RCU-protected (the RCU is only used to > access the vchiq_service). > > Signed-off-by: Adrien Thierry <athierry@xxxxxxxxxx> > --- > .../staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 8 +++----- > 1 file changed, 3 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 3bcb893d14a1..ba1f86799b7f 100644 > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > @@ -1058,6 +1058,9 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason, > > DEBUG_TRACE(SERVICE_CALLBACK_LINE); > > + if (!instance || instance->closing) > + return VCHIQ_SUCCESS; > + How can instance ever be NULL? And closing needs to be checked under a lock, right? thanks, greg k-h