Re: [PATCH 10/29] drivers, md: convert stripe_head.count from atomic_t to refcount_t

[Shaohua Li]

On Mon, Mar 06, 2017 at 04:20:57PM +0200, Elena Reshetova wrote:
> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.
> 
> Signed-off-by: Elena Reshetova 
> Signed-off-by: Hans Liljestrand 
> Signed-off-by: Kees Cook 
> Signed-off-by: David Windsor 
> ---
>  drivers/md/raid5-cache.c |  8 +++---
>  drivers/md/raid5.c   | 66 
> 
>  drivers/md/raid5.h   |  3 ++-
>  3 files changed, 39 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
> index 3f307be..6c05e12 100644
> --- a/drivers/md/raid5-cache.c
> +++ b/drivers/md/raid5-cache.c

snip
>  sh->check_state, sh->reconstruct_state);
>  
>   analyse_stripe(sh, );
> @@ -4924,7 +4924,7 @@ static void activate_bit_delay(struct r5conf *conf,
>   struct stripe_head *sh = list_entry(head.next, struct 
> stripe_head, lru);
>   int hash;
>   list_del_init(>lru);
> - atomic_inc(>count);
> + refcount_inc(>count);
>   hash = sh->hash_lock_index;
>   __release_stripe(conf, sh, _inactive_list[hash]);
>   }
> @@ -5240,7 +5240,7 @@ static struct stripe_head *__get_priority_stripe(struct 
> r5conf *conf, int group)
>   sh->group = NULL;
>   }
>   list_del_init(>lru);
> - BUG_ON(atomic_inc_return(>count) != 1);
> + BUG_ON(refcount_inc_not_zero(>count));

This changes the behavior. refcount_inc_not_zero doesn't inc if original value 
is 0

Thanks,
Shaohua

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t

[Shaohua Li]

On Mon, Mar 06, 2017 at 04:20:55PM +0200, Elena Reshetova wrote:
> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.

Looks good. Let me know how do you want to route the patch to upstream.
 
> Signed-off-by: Elena Reshetova 
> Signed-off-by: Hans Liljestrand 
> Signed-off-by: Kees Cook 
> Signed-off-by: David Windsor 
> ---
>  drivers/md/md.c | 6 +++---
>  drivers/md/md.h | 3 ++-
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 985374f..94c8ebf 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -449,7 +449,7 @@ EXPORT_SYMBOL(md_unplug);
>  
>  static inline struct mddev *mddev_get(struct mddev *mddev)
>  {
> - atomic_inc(>active);
> + refcount_inc(>active);
>   return mddev;
>  }
>  
> @@ -459,7 +459,7 @@ static void mddev_put(struct mddev *mddev)
>  {
>   struct bio_set *bs = NULL;
>  
> - if (!atomic_dec_and_lock(>active, _mddevs_lock))
> + if (!refcount_dec_and_lock(>active, _mddevs_lock))
>   return;
>   if (!mddev->raid_disks && list_empty(>disks) &&
>   mddev->ctime == 0 && !mddev->hold_active) {
> @@ -495,7 +495,7 @@ void mddev_init(struct mddev *mddev)
>   INIT_LIST_HEAD(>all_mddevs);
>   setup_timer(>safemode_timer, md_safemode_timeout,
>   (unsigned long) mddev);
> - atomic_set(>active, 1);
> + refcount_set(>active, 1);
>   atomic_set(>openers, 0);
>   atomic_set(>active_io, 0);
>   spin_lock_init(>lock);
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index b8859cb..4811663 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -360,7 +361,7 @@ struct mddev {
>*/
>   struct mutexopen_mutex;
>   struct mutexreconfig_mutex;
> - atomic_tactive; /* general refcount */
> + refcount_t  active; /* general refcount */
>   atomic_topeners;/* number of active 
> opens */
>  
>   int changed;/* True if we might 
> need to
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.