[systemd-devel] [PATCH 09/12] policy: kdbus_policy_set() use another variable to save entries

[Daniel Mack]

On 06/20/2014 06:50 PM, Djalal Harouni wrote:
> In kdbus_policy_set() function, we use the 'e' variable to reference
> each entry of the 'db->entries_hash', so at the end the variable 'e' will
> for sure point to a valid one.
> 
> Next in the KDBUS_ITEMS_FOREACH() iterator and if we fail at the first
> KDBUS_ITEM_VALID() test, we jmp to exit:
> 
> Which contains the following:
> if (e)
> 	kdbus_policy_entry_free(e);

This one looks right, but it depends on patch #7.

So the only missing patches from this series are #7 and #9 now.


Daniel


> 
> Here 'e' points to a valid entry and it will be freed, so even we
> restore all the other entries from that list, there will be always one
> missing, the last one pointed by that 'e' variable.
> 
> To fix this, just use another 'tmp_entry' variable to reference hash
> entries.
> 
> Signed-off-by: Djalal Harouni <tixxdz at opendz.org>
> ---
>  policy.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/policy.c b/policy.c
> index 9cf7f67..601d2a8 100644
> --- a/policy.c
> +++ b/policy.c
> @@ -467,6 +467,7 @@ int kdbus_policy_set(struct kdbus_policy_db *db,
>  		     const void *owner)
>  {
>  	struct kdbus_policy_db_entry *e = NULL;
> +	struct kdbus_policy_db_entry *tmp_entry = NULL;
>  	struct kdbus_policy_db_entry_access *a;
>  	const struct kdbus_item *item;
>  	struct hlist_node *tmp;
> @@ -483,8 +484,8 @@ int kdbus_policy_set(struct kdbus_policy_db *db,
>  	 * At the same time, the lookup mechanism won't find any collisions
>  	 * when looking for already exising names.
>  	 */
> -	hash_for_each_safe(db->entries_hash, i, tmp, e, hentry)
> -		if (e->owner == owner) {
> +	hash_for_each_safe(db->entries_hash, i, tmp, tmp_entry, hentry)
> +		if (tmp_entry->owner == owner) {
>  			struct kdbus_policy_list_entry *l;
>  
>  			l = kzalloc(sizeof(*l), GFP_KERNEL);
> @@ -493,9 +494,9 @@ int kdbus_policy_set(struct kdbus_policy_db *db,
>  				goto exit;
>  			}
>  
> -			l->e = e;
> +			l->e = tmp_entry;
>  			list_add_tail(&l->entry, &list);
> -			hash_del(&e->hentry);
> +			hash_del(&tmp_entry->hentry);
>  		}
>  
>  	/* Walk the list of items and look for new policies */
>