Re: [PATCH] omap: resource: Fix race in register_resource()

[Kevin Hilman <khilman@deeprootsystems.com>]

Mike Chan <mike@android.com> writes:

> Checking if the resource is already registered and adding to the list
> must be atomic or bad things can happen.
>
> Signed-off-by: Mike Chan <mike@android.com>
> ---
>  arch/arm/plat-omap/resource.c |   13 ++++++++-----
>  1 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
> index 111020a..4631912 100644
> --- a/arch/arm/plat-omap/resource.c
> +++ b/arch/arm/plat-omap/resource.c
> @@ -255,6 +255,7 @@ int resource_refresh(void)
>   */
>  int resource_register(struct shared_resource *resp)
>  {
> +	int ret = 0;
>  	if (!resp)
>  		return -EINVAL;
>  
> @@ -262,12 +263,13 @@ int resource_register(struct shared_resource *resp)
>  		return -EINVAL;
>  
>  	/* Verify that the resource is not already registered */
> -	if (resource_lookup(resp->name))
> -		return -EEXIST;
> +	down(&res_mutex);
> +	if (_resource_lookup(resp->name))
> +		ret = -EEXIST;
> +		goto out;

Ahem, you're rebased version has a little problem.  Missing {} block
mans this goto is always executed and no resources are ever added.

I'll fix this up manually before applying, but some more testing next
time would be helpful

Thanks,

Kevin


>  	INIT_LIST_HEAD(&resp->users_list);
>  
> -	down(&res_mutex);
>  	/* Add the resource to the resource list */
>  	list_add(&resp->node, &res_list);
>  
> @@ -275,10 +277,11 @@ int resource_register(struct shared_resource *resp)
>  	if (resp->ops->init)
>  		resp->ops->init(resp);
>  
> -	up(&res_mutex);
>  	pr_debug("resource: registered %s\n", resp->name);
>  
> -	return 0;
> +out:
> +	up(&res_mutex);
> +	return ret;
>  }
>  EXPORT_SYMBOL(resource_register);
>  
> -- 
> 1.5.4.5