* Re: Linux 3.16.54: suspend stops working. [not found] <02c917a9-83ad-6510-fe7d-9ab8b4519041@yahoo.com> @ 2018-02-19 15:18 ` Takashi Iwai [not found] ` <2f37c140-3cd4-e1e6-9c41-631b637d4883@yahoo.com> 0 siblings, 1 reply; 4+ messages in thread From: Takashi Iwai @ 2018-02-19 15:18 UTC (permalink / raw) To: Andres Bertens; +Cc: linux-kernel, stable, ben On Mon, 19 Feb 2018 16:06:55 +0100, Andres Bertens wrote: > > Hi there! > > If of any interest to you, upgrading to linux 3.16.54 from 3.16.53, > suspend stops working. The same is for upgrading from linux 4.4.114 to > 4.4.115. > > Checking pm-utils log: > > /usr/lib/pm-utils/pm-functions: line 259: echo: write error: Resource > temporarily unavailable > > And line 259 is: > do_suspend() { echo -n "mem" >/sys/power/state; } > > > > Reverting patch: > commit 02cbce8576a31df8fca54aaec91ee081076bd79d > Author: Takashi Iwai <tiwai@suse.de> > Date: Tue Jan 9 23:11:03 2018 +0100 > > ALSA: seq: Make ioctls race-free > > commit b3defb791b26ea0683a93a4f49c77ec45ec96f10 upstream. > > makes it work again! Just a blind shot: could you try the patch below instead? And, are you using some sequencer audio stuff? thanks, Takashi -- 8< -- From: Takashi Iwai <tiwai@suse.de> Subject: [PATCH] ALSA: seq: Make ioctls race-free (revised) The ALSA sequencer ioctls have no protection against racy calls while the concurrent operations may lead to interfere with each other. As reported recently, for example, the concurrent calls of setting client pool with a combination of write calls may lead to either the unkillable dead-lock or UAF. As a slightly big hammer solution, this patch introduces the mutex to make each ioctl exclusive. Although this may reduce performance via parallel ioctl calls, usually it's not demanded for sequencer usages, hence it should be negligible. Reported-by: Luo Quan <a4651386@163.com> Reviewed-by: Kees Cook <keescook@chromium.org> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: <stable@vger.kernel.org> Signed-off-by: Takashi Iwai <tiwai@suse.de> --- sound/core/seq/seq_clientmgr.c | 7 ++++++- sound/core/seq/seq_clientmgr.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -236,6 +236,7 @@ static struct snd_seq_client *seq_create rwlock_init(&client->ports_lock); mutex_init(&client->ports_mutex); INIT_LIST_HEAD(&client->ports_list_head); + mutex_init(&client->ioctl_mutex); /* find free slot in the client table */ spin_lock_irqsave(&clients_lock, flags); @@ -2220,11 +2221,15 @@ static int snd_seq_do_ioctl(struct snd_s static long snd_seq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { struct snd_seq_client *client = file->private_data; + long ret; if (snd_BUG_ON(!client)) return -ENXIO; - return snd_seq_do_ioctl(client, cmd, (void __user *) arg); + mutex_lock(&client->ioctl_mutex); + ret = snd_seq_do_ioctl(client, cmd, (void __user *) arg); + mutex_unlock(&client->ioctl_mutex); + return ret; } #ifdef CONFIG_COMPAT --- a/sound/core/seq/seq_clientmgr.h +++ b/sound/core/seq/seq_clientmgr.h @@ -59,6 +59,7 @@ struct snd_seq_client { struct list_head ports_list_head; rwlock_t ports_lock; struct mutex ports_mutex; + struct mutex ioctl_mutex; int convert32; /* convert 32->64bit */ /* output pool */ ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <2f37c140-3cd4-e1e6-9c41-631b637d4883@yahoo.com>]
* Re: Linux 3.16.54: suspend stops working. [not found] ` <2f37c140-3cd4-e1e6-9c41-631b637d4883@yahoo.com> @ 2018-02-19 15:46 ` Takashi Iwai 2018-02-19 15:55 ` Greg Kroah-Hartman 0 siblings, 1 reply; 4+ messages in thread From: Takashi Iwai @ 2018-02-19 15:46 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Andres Bertens, linux-kernel, stable, ben On Mon, 19 Feb 2018 16:36:03 +0100, Andres Bertens wrote: > > Hi, > > Yes sir, supend works with the attached patch! > > As for using some sequencer audio stuff, I have enabled > CONFIG_SND_SEQUENCER and some others *SEQ* as modules in my kernel > config. Nothing else to my knowledge. Good to hear! Greg, could you revert the previous one and apply the new one instead? Below is with a more description explaining the difference. Or if you prefer an incremental patch instead of revert and re-apply, let me know. I'll cook it. thanks, Takashi -- 8< -- From: Takashi Iwai <tiwai@suse.de> Subject: [PATCH] ALSA: seq: Make ioctls race-free (revised) commit b3defb791b26ea0683a93a4f49c77ec45ec96f10 upstream. The ALSA sequencer ioctls have no protection against racy calls while the concurrent operations may lead to interfere with each other. As reported recently, for example, the concurrent calls of setting client pool with a combination of write calls may lead to either the unkillable dead-lock or UAF. As a slightly big hammer solution, this patch introduces the mutex to make each ioctl exclusive. Although this may reduce performance via parallel ioctl calls, usually it's not demanded for sequencer usages, hence it should be negligible. [ NOTE: this is the revised backport of the commit b3defb791b26. We had another backport (e.g. 623e5c8ae32b in 4.4.115), but it applies the new mutex also to the code paths via faked kernel-to-kernel ioctls, and it seems leading to a deadlock, as reported recently as a regression. This revised patch takes the mutex only in the code path invoked by user-space, just like the original fix patch does -- tiwai ] Reported-by: Luo Quan <a4651386@163.com> Reviewed-by: Kees Cook <keescook@chromium.org> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: <stable@vger.kernel.org> Signed-off-by: Takashi Iwai <tiwai@suse.de> --- sound/core/seq/seq_clientmgr.c | 7 ++++++- sound/core/seq/seq_clientmgr.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -236,6 +236,7 @@ static struct snd_seq_client *seq_create rwlock_init(&client->ports_lock); mutex_init(&client->ports_mutex); INIT_LIST_HEAD(&client->ports_list_head); + mutex_init(&client->ioctl_mutex); /* find free slot in the client table */ spin_lock_irqsave(&clients_lock, flags); @@ -2220,11 +2221,15 @@ static int snd_seq_do_ioctl(struct snd_s static long snd_seq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { struct snd_seq_client *client = file->private_data; + long ret; if (snd_BUG_ON(!client)) return -ENXIO; - return snd_seq_do_ioctl(client, cmd, (void __user *) arg); + mutex_lock(&client->ioctl_mutex); + ret = snd_seq_do_ioctl(client, cmd, (void __user *) arg); + mutex_unlock(&client->ioctl_mutex); + return ret; } #ifdef CONFIG_COMPAT --- a/sound/core/seq/seq_clientmgr.h +++ b/sound/core/seq/seq_clientmgr.h @@ -59,6 +59,7 @@ struct snd_seq_client { struct list_head ports_list_head; rwlock_t ports_lock; struct mutex ports_mutex; + struct mutex ioctl_mutex; int convert32; /* convert 32->64bit */ /* output pool */ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Linux 3.16.54: suspend stops working. 2018-02-19 15:46 ` Takashi Iwai @ 2018-02-19 15:55 ` Greg Kroah-Hartman 2018-02-19 15:57 ` Takashi Iwai 0 siblings, 1 reply; 4+ messages in thread From: Greg Kroah-Hartman @ 2018-02-19 15:55 UTC (permalink / raw) To: Takashi Iwai; +Cc: Andres Bertens, linux-kernel, stable, ben On Mon, Feb 19, 2018 at 04:46:29PM +0100, Takashi Iwai wrote: > On Mon, 19 Feb 2018 16:36:03 +0100, > Andres Bertens wrote: > > > > Hi, > > > > Yes sir, supend works with the attached patch! > > > > As for using some sequencer audio stuff, I have enabled > > CONFIG_SND_SEQUENCER and some others *SEQ* as modules in my kernel > > config. Nothing else to my knowledge. > > Good to hear! > > Greg, could you revert the previous one and apply the new one instead? > Below is with a more description explaining the difference. > > Or if you prefer an incremental patch instead of revert and re-apply, > let me know. I'll cook it. Incremental would be best, but I don't handle 3.16.y, Ben does :) thanks, greg k-h ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Linux 3.16.54: suspend stops working. 2018-02-19 15:55 ` Greg Kroah-Hartman @ 2018-02-19 15:57 ` Takashi Iwai 0 siblings, 0 replies; 4+ messages in thread From: Takashi Iwai @ 2018-02-19 15:57 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Andres Bertens, linux-kernel, stable, ben On Mon, 19 Feb 2018 16:55:54 +0100, Greg Kroah-Hartman wrote: > > On Mon, Feb 19, 2018 at 04:46:29PM +0100, Takashi Iwai wrote: > > On Mon, 19 Feb 2018 16:36:03 +0100, > > Andres Bertens wrote: > > > > > > Hi, > > > > > > Yes sir, supend works with the attached patch! > > > > > > As for using some sequencer audio stuff, I have enabled > > > CONFIG_SND_SEQUENCER and some others *SEQ* as modules in my kernel > > > config. Nothing else to my knowledge. > > > > Good to hear! > > > > Greg, could you revert the previous one and apply the new one instead? > > Below is with a more description explaining the difference. > > > > Or if you prefer an incremental patch instead of revert and re-apply, > > let me know. I'll cook it. > > Incremental would be best, but I don't handle 3.16.y, Ben does :) This hits 4.4.x, too, as it contains the same backport. The backports for newer stable kernels are fine. In anyway, I'm going to send a fix-up patch, then. thanks, Takashi ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-02-19 15:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <02c917a9-83ad-6510-fe7d-9ab8b4519041@yahoo.com>
2018-02-19 15:18 ` Linux 3.16.54: suspend stops working Takashi Iwai
[not found] ` <2f37c140-3cd4-e1e6-9c41-631b637d4883@yahoo.com>
2018-02-19 15:46 ` Takashi Iwai
2018-02-19 15:55 ` Greg Kroah-Hartman
2018-02-19 15:57 ` Takashi Iwai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox