* [PATCH] sound: convert snd_seq_subscribers.ref_count from atomic_t to refcount_t
@ 2017-02-21 15:27 Elena Reshetova
2017-02-21 15:42 ` Peter Zijlstra
2017-02-23 14:55 ` Takashi Iwai
0 siblings, 2 replies; 3+ messages in thread
From: Elena Reshetova @ 2017-02-21 15:27 UTC (permalink / raw)
To: linux-kernel
Cc: alsa-devel, peterz, gregkh, perex, tiwai, Elena Reshetova,
Hans Liljestrand, Kees Cook, David Windsor
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 <elena.reshetova@intel.com>
Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David Windsor <dwindsor@gmail.com>
---
sound/core/seq/seq_clientmgr.c | 2 +-
sound/core/seq/seq_ports.c | 7 +++----
sound/core/seq/seq_ports.h | 3 ++-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 4c93520..3440f76 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -666,7 +666,7 @@ static int deliver_to_subscribers(struct snd_seq_client *client,
down_read(&grp->list_mutex);
list_for_each_entry(subs, &grp->list_head, src_list) {
/* both ports ready? */
- if (atomic_read(&subs->ref_count) != 2)
+ if (refcount_read(&subs->ref_count) != 2)
continue;
event->dest = subs->info.dest;
if (subs->info.flags & SNDRV_SEQ_PORT_SUBS_TIMESTAMP)
diff --git a/sound/core/seq/seq_ports.c b/sound/core/seq/seq_ports.c
index fe686ee..1ca896b 100644
--- a/sound/core/seq/seq_ports.c
+++ b/sound/core/seq/seq_ports.c
@@ -241,7 +241,7 @@ static void clear_subscriber_list(struct snd_seq_client *client,
* we decrease the counter, and when both ports are deleted
* remove the subscriber info
*/
- if (atomic_dec_and_test(&subs->ref_count))
+ if (refcount_dec_and_test(&subs->ref_count))
kfree(subs);
continue;
}
@@ -520,7 +520,6 @@ static int check_and_subscribe_port(struct snd_seq_client *client,
else
list_add_tail(&subs->dest_list, &grp->list_head);
grp->exclusive = exclusive;
- atomic_inc(&subs->ref_count);
write_unlock_irq(&grp->list_lock);
err = 0;
@@ -570,7 +569,6 @@ int snd_seq_port_connect(struct snd_seq_client *connector,
return -ENOMEM;
subs->info = *info;
- atomic_set(&subs->ref_count, 0);
INIT_LIST_HEAD(&subs->src_list);
INIT_LIST_HEAD(&subs->dest_list);
@@ -587,6 +585,7 @@ int snd_seq_port_connect(struct snd_seq_client *connector,
if (err < 0)
goto error_dest;
+ refcount_set(&subs->ref_count, 2);
return 0;
error_dest:
@@ -613,7 +612,7 @@ int snd_seq_port_disconnect(struct snd_seq_client *connector,
/* look for the connection */
list_for_each_entry(subs, &src->list_head, src_list) {
if (match_subs_info(info, &subs->info)) {
- atomic_dec(&subs->ref_count); /* mark as not ready */
+ refcount_dec(&subs->ref_count); /* mark as not ready */
err = 0;
break;
}
diff --git a/sound/core/seq/seq_ports.h b/sound/core/seq/seq_ports.h
index 26bd71f..d09e396 100644
--- a/sound/core/seq/seq_ports.h
+++ b/sound/core/seq/seq_ports.h
@@ -21,6 +21,7 @@
#ifndef __SND_SEQ_PORTS_H
#define __SND_SEQ_PORTS_H
+#include <linux/refcount.h>
#include <sound/seq_kernel.h>
#include "seq_lock.h"
@@ -44,7 +45,7 @@ struct snd_seq_subscribers {
struct snd_seq_port_subscribe info; /* additional info */
struct list_head src_list; /* link of sources */
struct list_head dest_list; /* link of destinations */
- atomic_t ref_count;
+ refcount_t ref_count;
};
struct snd_seq_port_subs_info {
--
2.7.4
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] sound: convert snd_seq_subscribers.ref_count from atomic_t to refcount_t
2017-02-21 15:27 [PATCH] sound: convert snd_seq_subscribers.ref_count from atomic_t to refcount_t Elena Reshetova
@ 2017-02-21 15:42 ` Peter Zijlstra
2017-02-23 14:55 ` Takashi Iwai
1 sibling, 0 replies; 3+ messages in thread
From: Peter Zijlstra @ 2017-02-21 15:42 UTC (permalink / raw)
To: Elena Reshetova
Cc: linux-kernel, alsa-devel, gregkh, perex, tiwai, Hans Liljestrand,
Kees Cook, David Windsor
On Tue, Feb 21, 2017 at 05:27:10PM +0200, Elena Reshetova wrote:
> diff --git a/sound/core/seq/seq_ports.c b/sound/core/seq/seq_ports.c
> index fe686ee..1ca896b 100644
> --- a/sound/core/seq/seq_ports.c
> +++ b/sound/core/seq/seq_ports.c
> @@ -520,7 +520,6 @@ static int check_and_subscribe_port(struct snd_seq_client *client,
> else
> list_add_tail(&subs->dest_list, &grp->list_head);
> grp->exclusive = exclusive;
> - atomic_inc(&subs->ref_count);
> write_unlock_irq(&grp->list_lock);
> err = 0;
>
> @@ -570,7 +569,6 @@ int snd_seq_port_connect(struct snd_seq_client *connector,
> return -ENOMEM;
>
> subs->info = *info;
> - atomic_set(&subs->ref_count, 0);
> INIT_LIST_HEAD(&subs->src_list);
> INIT_LIST_HEAD(&subs->dest_list);
>
> @@ -587,6 +585,7 @@ int snd_seq_port_connect(struct snd_seq_client *connector,
> if (err < 0)
> goto error_dest;
>
> + refcount_set(&subs->ref_count, 2);
> return 0;
>
> error_dest:
> @@ -613,7 +612,7 @@ int snd_seq_port_disconnect(struct snd_seq_client *connector,
> /* look for the connection */
> list_for_each_entry(subs, &src->list_head, src_list) {
> if (match_subs_info(info, &subs->info)) {
> - atomic_dec(&subs->ref_count); /* mark as not ready */
> + refcount_dec(&subs->ref_count); /* mark as not ready */
> err = 0;
> break;
> }
This looks dodgy... someone should look hard at this.
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] sound: convert snd_seq_subscribers.ref_count from atomic_t to refcount_t
2017-02-21 15:27 [PATCH] sound: convert snd_seq_subscribers.ref_count from atomic_t to refcount_t Elena Reshetova
2017-02-21 15:42 ` Peter Zijlstra
@ 2017-02-23 14:55 ` Takashi Iwai
1 sibling, 0 replies; 3+ messages in thread
From: Takashi Iwai @ 2017-02-23 14:55 UTC (permalink / raw)
To: Elena Reshetova
Cc: linux-kernel, alsa-devel, Kees Cook, David Windsor,
Hans Liljestrand, peterz, gregkh, perex
On Tue, 21 Feb 2017 16:27:10 +0100,
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.
Well, in this case, it cannot overflow since the highest refcount is
2, as you already figured out.
Takashi
>
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: David Windsor <dwindsor@gmail.com>
> ---
> sound/core/seq/seq_clientmgr.c | 2 +-
> sound/core/seq/seq_ports.c | 7 +++----
> sound/core/seq/seq_ports.h | 3 ++-
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
> index 4c93520..3440f76 100644
> --- a/sound/core/seq/seq_clientmgr.c
> +++ b/sound/core/seq/seq_clientmgr.c
> @@ -666,7 +666,7 @@ static int deliver_to_subscribers(struct snd_seq_client *client,
> down_read(&grp->list_mutex);
> list_for_each_entry(subs, &grp->list_head, src_list) {
> /* both ports ready? */
> - if (atomic_read(&subs->ref_count) != 2)
> + if (refcount_read(&subs->ref_count) != 2)
> continue;
> event->dest = subs->info.dest;
> if (subs->info.flags & SNDRV_SEQ_PORT_SUBS_TIMESTAMP)
> diff --git a/sound/core/seq/seq_ports.c b/sound/core/seq/seq_ports.c
> index fe686ee..1ca896b 100644
> --- a/sound/core/seq/seq_ports.c
> +++ b/sound/core/seq/seq_ports.c
> @@ -241,7 +241,7 @@ static void clear_subscriber_list(struct snd_seq_client *client,
> * we decrease the counter, and when both ports are deleted
> * remove the subscriber info
> */
> - if (atomic_dec_and_test(&subs->ref_count))
> + if (refcount_dec_and_test(&subs->ref_count))
> kfree(subs);
> continue;
> }
> @@ -520,7 +520,6 @@ static int check_and_subscribe_port(struct snd_seq_client *client,
> else
> list_add_tail(&subs->dest_list, &grp->list_head);
> grp->exclusive = exclusive;
> - atomic_inc(&subs->ref_count);
> write_unlock_irq(&grp->list_lock);
> err = 0;
>
> @@ -570,7 +569,6 @@ int snd_seq_port_connect(struct snd_seq_client *connector,
> return -ENOMEM;
>
> subs->info = *info;
> - atomic_set(&subs->ref_count, 0);
> INIT_LIST_HEAD(&subs->src_list);
> INIT_LIST_HEAD(&subs->dest_list);
>
> @@ -587,6 +585,7 @@ int snd_seq_port_connect(struct snd_seq_client *connector,
> if (err < 0)
> goto error_dest;
>
> + refcount_set(&subs->ref_count, 2);
> return 0;
>
> error_dest:
> @@ -613,7 +612,7 @@ int snd_seq_port_disconnect(struct snd_seq_client *connector,
> /* look for the connection */
> list_for_each_entry(subs, &src->list_head, src_list) {
> if (match_subs_info(info, &subs->info)) {
> - atomic_dec(&subs->ref_count); /* mark as not ready */
> + refcount_dec(&subs->ref_count); /* mark as not ready */
> err = 0;
> break;
> }
> diff --git a/sound/core/seq/seq_ports.h b/sound/core/seq/seq_ports.h
> index 26bd71f..d09e396 100644
> --- a/sound/core/seq/seq_ports.h
> +++ b/sound/core/seq/seq_ports.h
> @@ -21,6 +21,7 @@
> #ifndef __SND_SEQ_PORTS_H
> #define __SND_SEQ_PORTS_H
>
> +#include <linux/refcount.h>
> #include <sound/seq_kernel.h>
> #include "seq_lock.h"
>
> @@ -44,7 +45,7 @@ struct snd_seq_subscribers {
> struct snd_seq_port_subscribe info; /* additional info */
> struct list_head src_list; /* link of sources */
> struct list_head dest_list; /* link of destinations */
> - atomic_t ref_count;
> + refcount_t ref_count;
> };
>
> struct snd_seq_port_subs_info {
> --
> 2.7.4
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-02-23 14:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-21 15:27 [PATCH] sound: convert snd_seq_subscribers.ref_count from atomic_t to refcount_t Elena Reshetova
2017-02-21 15:42 ` Peter Zijlstra
2017-02-23 14:55 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®