Hi Al
On Fri, Oct 31, 2014 at 12:38 AM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> On Wed, Oct 29, 2014 at 03:00:52PM -0700, Greg Kroah-Hartman wrote:
>
>> +static void __kdbus_domain_user_free(struct kref *kref)
>> +{
>> + struct kdbus_domain_user *user =
>> + container_of(kref, struct kdbus_domain_user, kref);
>> +
>> + BUG_ON(atomic_read(&user->buses) > 0);
>> + BUG_ON(atomic_read(&user->connections) > 0);
>> +
>> + mutex_lock(&user->domain->lock);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> + idr_remove(&user->domain->user_idr, user->idr);
>> + hash_del(&user->hentry);
> ^^^^^^^^^^^^^^^^^^^^^^^^
>> + mutex_unlock(&user->domain->lock);
>> +
>> + kdbus_domain_unref(user->domain);
>> + kfree(user);
>> +}
>
>> +struct kdbus_domain_user *kdbus_domain_user_unref(struct kdbus_domain_user *u)
>> +{
>> + if (u)
>> + kref_put(&u->kref, __kdbus_domain_user_free);
>> + return NULL;
>> +}
>
> If you remove an object from some search structures, taking the lock in
> destructor is Too Fucking Late(tm). Somebody might have already found
> that puppy and decided to pick it (all under that lock) just as we'd
> got to that point in destructor and blocked there. Oops...
Nice catch! I fixed it up via kref_get_unless_zero(). This has the
side-effect that there might be multiple domain_user objects for the
same user, but all but one will have ref==0. They don't carry and
valuable data in those cases, so we're fine. We will just end up using
the next one, or creating a new one.
Thanks a lot!
David
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
