[systemd-devel] [PATCH 10/12] policy: kdbus_policy_set() fix a use after free bug

[Daniel Mack]

On 06/20/2014 06:50 PM, Djalal Harouni wrote:
> If we try to register the same name twice then kdbus_policy_add_one()
> will fail with -EEXIST
> 
> In kdbus_policy_set() we have two calls to kdbus_policy_add_one() if
> they fail we clean things up with kdbus_policy_entry_free(), but since
> we failed ret == -EEXIST ,we hit the error path where we have another:
> 
> if (e)
> 	kdbus_policy_entry_free(e);
> 
> We have a use after free bug here, Since 'e' is freed but never set to
> NULL.

Applied, thanks.


Daniel


> 
> To fix this we can set that 'e' to NULL after each
> kdbus_policy_entry_free() call, but it is better to just clean things up
> in a one place, in the error path and remove the other
> kdbus_policy_entry_free() calls.
> 
> Thix fixes the bug triggered by test-kdbus-policy when we try to
> register the same name twice.
> 
> Signed-off-by: Djalal Harouni <tixxdz at opendz.org>
> ---
>  policy.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/policy.c b/policy.c
> index 601d2a8..d75c2ef 100644
> --- a/policy.c
> +++ b/policy.c
> @@ -512,10 +512,8 @@ int kdbus_policy_set(struct kdbus_policy_db *db,
>  
>  			if (e) {
>  				ret = kdbus_policy_add_one(db, e);
> -				if (ret < 0) {
> -					kdbus_policy_entry_free(e);
> +				if (ret < 0)
>  					goto exit;
> -				}
>  			}
>  
>  			if (max_policies && ++count > max_policies) {
> @@ -584,11 +582,8 @@ int kdbus_policy_set(struct kdbus_policy_db *db,
>  		goto exit;
>  	}
>  
> -	if (e) {
> +	if (e)
>  		ret = kdbus_policy_add_one(db, e);
> -		if (ret < 0)
> -			kdbus_policy_entry_free(e);
> -	}
>  
>  exit:
>  	if (ret < 0) {
>