When a service is removed from the state->services array in
unlock_service(), its spot in the array is set to NULL, and then it is
freed. And there is code in many places that does something like:
struct vchiq_service *service = state->services[i];
if (service && service->someting && service->something_else)
...
But the problem is that this could race with unlock_serivce(), where
we read a non-null value right before it gets set to NULL and
freed. This would be ok if the code above had an active ref_count on
the service, but that's often not the case. So this patch is a
proposal to reimplement the reference counting to use a struct kref,
and to use kfree_rcu() instead of kfree() when freeing the services,
so that the code above will not be buggy under rcu_read_lock(). It
seemed like the right choice because there are lots of places where
the above pattern exists and the caller doesn't have a reference, and
doesn't need to keep one. And in several places this is done in a loop
over all services.
I tested this with the vchiq_test program and also with a program that
spins creating/deleting services, but more help/advice is appreciated.
I think there's still a related race condition where a struct
vchiq_instance is dereferenced without a guarantee that it won't go
away. In vchiq_dump_platform_instances(), for example,
service->instance is dereferenced a bunch, but someone else could
close it in the middle. Also in vchiq_blocking_bulk_transfer() it
seems possible that someone else closes it after reading
service->instance? I might be missing something but after looking
around for a bit I couldn't see any reason the instance would wait for
this function to complete before being kfree'd.
Marcelo Diop-Gonzalez (5):
staging: vc04_services: remove unused function
staging: vc04_services: remove unneeded parentheses
staging: vc04_services: fix indentation alignment in a few places
staging: vc04_services: use kref + RCU to reference count services
staging: vc04_services: don't increment service refcount when it's not
needed
.../interface/vchiq_arm/vchiq_arm.c | 41 ++-
.../interface/vchiq_arm/vchiq_core.c | 286 +++++++++---------
.../interface/vchiq_arm/vchiq_core.h | 20 +-
.../interface/vchiq_arm/vchiq_if.h | 2 -
4 files changed, 190 insertions(+), 159 deletions(-)
--
2.25.0.225.g125e21ebc7-goog
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
