drivers/char/hw_random/core.c | 30 ++++++++++++++++++++++++---- drivers/net/wireless/ath/ath9k/rng.c | 19 +++++++----------- kernel/sched/core.c | 1 + 3 files changed, 34 insertions(+), 16 deletions(-)
There are two deadlock scenarios that need addressing, which cause
problems when the computer goes to sleep, the interface is set down, and
hwrng_unregister() is called. When the deadlock is hit, sleep is delayed
for tens of seconds, causing it to fail. These scenarios are:
1) The hwrng kthread can't be stopped while it's sleeping, because it
uses msleep_interruptible() instead of schedule_timeout_interruptible().
The fix is a simple moving to the correct function. At the same time,
we should cleanup a common and useless dmesg splat in the same area.
2) A normal user thread can't be interrupted by hwrng_unregister() while
it's sleeping, because hwrng_unregister() is called from elsewhere.
The solution here is to keep track of which thread is currently
reading, and asleep, and signal that thread when it's time to
unregister. There's a bit of book keeping required to prevent
lifetime issues on current.
Reported-by: Gregory Erwin <gregerwin256@gmail.com>
Cc: Toke Høiland-Jørgensen <toke@redhat.com>
Cc: Kalle Valo <kvalo@kernel.org>
Cc: Rui Salvaterra <rsalvaterra@gmail.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: stable@vger.kernel.org
Fixes: fcd09c90c3c5 ("ath9k: use hw_random API instead of directly dumping into random.c")
Link: https://lore.kernel.org/all/CAO+Okf6ZJC5-nTE_EJUGQtd8JiCkiEHytGgDsFGTEjs0c00giw@mail.gmail.com/
Link: https://lore.kernel.org/lkml/CAO+Okf5k+C+SE6pMVfPf-d8MfVPVq4PO7EY8Hys_DVXtent3HA@mail.gmail.com/
Link: https://bugs.archlinux.org/task/75138
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
Changes v7->v8:
- Add a missing export_symbol.
drivers/char/hw_random/core.c | 30 ++++++++++++++++++++++++----
drivers/net/wireless/ath/ath9k/rng.c | 19 +++++++-----------
kernel/sched/core.c | 1 +
3 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 16f227b995e8..df45c265878e 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -38,6 +38,8 @@ static LIST_HEAD(rng_list);
static DEFINE_MUTEX(rng_mutex);
/* Protects rng read functions, data_avail, rng_buffer and rng_fillbuf */
static DEFINE_MUTEX(reading_mutex);
+/* Keeps track of whoever is wait-reading it currently while holding reading_mutex. */
+static struct task_struct *current_waiting_reader;
static int data_avail;
static u8 *rng_buffer, *rng_fillbuf;
static unsigned short current_quality;
@@ -208,6 +210,7 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf,
int err = 0;
int bytes_read, len;
struct hwrng *rng;
+ bool wait;
while (size) {
rng = get_current_rng();
@@ -225,9 +228,15 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf,
goto out_put;
}
if (!data_avail) {
+ wait = !(filp->f_flags & O_NONBLOCK);
+ if (wait && cmpxchg(¤t_waiting_reader, NULL, current) != NULL) {
+ err = -EINTR;
+ goto out_unlock_reading;
+ }
bytes_read = rng_get_data(rng, rng_buffer,
- rng_buffer_size(),
- !(filp->f_flags & O_NONBLOCK));
+ rng_buffer_size(), wait);
+ if (wait && cmpxchg(¤t_waiting_reader, current, NULL) != current)
+ synchronize_rcu();
if (bytes_read < 0) {
err = bytes_read;
goto out_unlock_reading;
@@ -513,8 +522,9 @@ static int hwrng_fillfn(void *unused)
break;
if (rc <= 0) {
- pr_warn("hwrng: no data available\n");
- msleep_interruptible(10000);
+ if (kthread_should_stop())
+ break;
+ schedule_timeout_interruptible(HZ * 10);
continue;
}
@@ -608,13 +618,21 @@ int hwrng_register(struct hwrng *rng)
}
EXPORT_SYMBOL_GPL(hwrng_register);
+#define UNREGISTERING_READER ((void *)~0UL)
+
void hwrng_unregister(struct hwrng *rng)
{
struct hwrng *old_rng, *new_rng;
+ struct task_struct *waiting_reader;
int err;
mutex_lock(&rng_mutex);
+ rcu_read_lock();
+ waiting_reader = xchg(¤t_waiting_reader, UNREGISTERING_READER);
+ if (waiting_reader && waiting_reader != UNREGISTERING_READER)
+ set_notify_signal(waiting_reader);
+ rcu_read_unlock();
old_rng = current_rng;
list_del(&rng->list);
if (current_rng == rng) {
@@ -640,6 +658,10 @@ void hwrng_unregister(struct hwrng *rng)
}
wait_for_completion(&rng->cleanup_done);
+
+ mutex_lock(&rng_mutex);
+ cmpxchg(¤t_waiting_reader, UNREGISTERING_READER, NULL);
+ mutex_unlock(&rng_mutex);
}
EXPORT_SYMBOL_GPL(hwrng_unregister);
diff --git a/drivers/net/wireless/ath/ath9k/rng.c b/drivers/net/wireless/ath/ath9k/rng.c
index cb5414265a9b..8980dc36509e 100644
--- a/drivers/net/wireless/ath/ath9k/rng.c
+++ b/drivers/net/wireless/ath/ath9k/rng.c
@@ -52,18 +52,13 @@ static int ath9k_rng_data_read(struct ath_softc *sc, u32 *buf, u32 buf_size)
return j << 2;
}
-static u32 ath9k_rng_delay_get(u32 fail_stats)
+static unsigned long ath9k_rng_delay_get(u32 fail_stats)
{
- u32 delay;
-
if (fail_stats < 100)
- delay = 10;
+ return HZ / 100;
else if (fail_stats < 105)
- delay = 1000;
- else
- delay = 10000;
-
- return delay;
+ return HZ;
+ return HZ * 10;
}
static int ath9k_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
@@ -80,10 +75,10 @@ static int ath9k_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
bytes_read += max & 3UL;
memzero_explicit(&word, sizeof(word));
}
- if (!wait || !max || likely(bytes_read) || fail_stats > 110)
+ if (!wait || !max || likely(bytes_read) || fail_stats > 110 ||
+ ((current->flags & PF_KTHREAD) && kthread_should_stop()) ||
+ schedule_timeout_interruptible(ath9k_rng_delay_get(++fail_stats)))
break;
-
- msleep_interruptible(ath9k_rng_delay_get(++fail_stats));
}
if (wait && !bytes_read && max)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index da0bf6fe9ecd..d65a5eb9a65e 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4284,6 +4284,7 @@ int wake_up_state(struct task_struct *p, unsigned int state)
{
return try_to_wake_up(p, state, 0);
}
+EXPORT_SYMBOL(wake_up_state);
/*
* Perform scheduler related setup for a newly forked process p.
--
2.35.1
"Jason A. Donenfeld" <Jason@zx2c4.com> writes:
> There are two deadlock scenarios that need addressing, which cause
> problems when the computer goes to sleep, the interface is set down, and
> hwrng_unregister() is called. When the deadlock is hit, sleep is delayed
> for tens of seconds, causing it to fail. These scenarios are:
>
> 1) The hwrng kthread can't be stopped while it's sleeping, because it
> uses msleep_interruptible() instead of schedule_timeout_interruptible().
> The fix is a simple moving to the correct function. At the same time,
> we should cleanup a common and useless dmesg splat in the same area.
>
> 2) A normal user thread can't be interrupted by hwrng_unregister() while
> it's sleeping, because hwrng_unregister() is called from elsewhere.
> The solution here is to keep track of which thread is currently
> reading, and asleep, and signal that thread when it's time to
> unregister. There's a bit of book keeping required to prevent
> lifetime issues on current.
>
> Reported-by: Gregory Erwin <gregerwin256@gmail.com>
> Cc: Toke Høiland-Jørgensen <toke@redhat.com>
> Cc: Kalle Valo <kvalo@kernel.org>
> Cc: Rui Salvaterra <rsalvaterra@gmail.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: stable@vger.kernel.org
> Fixes: fcd09c90c3c5 ("ath9k: use hw_random API instead of directly dumping into random.c")
> Link: https://lore.kernel.org/all/CAO+Okf6ZJC5-nTE_EJUGQtd8JiCkiEHytGgDsFGTEjs0c00giw@mail.gmail.com/
> Link: https://lore.kernel.org/lkml/CAO+Okf5k+C+SE6pMVfPf-d8MfVPVq4PO7EY8Hys_DVXtent3HA@mail.gmail.com/
> Link: https://bugs.archlinux.org/task/75138
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> Changes v7->v8:
> - Add a missing export_symbol.
>
> drivers/char/hw_random/core.c | 30 ++++++++++++++++++++++++----
> drivers/net/wireless/ath/ath9k/rng.c | 19 +++++++-----------
> kernel/sched/core.c | 1 +
> 3 files changed, 34 insertions(+), 16 deletions(-)
I don't see any acks for the hw_random and the scheduler change, adding more
people to CC. Full patch here:
https://patchwork.kernel.org/project/linux-wireless/patch/20220629114240.946411-1-Jason@zx2c4.com/
Are everyone ok if I take this patch via wireless-next?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 07/07/22 19:26, Kalle Valo wrote:
> "Jason A. Donenfeld" <Jason@zx2c4.com> writes:
>
>> There are two deadlock scenarios that need addressing, which cause
>> problems when the computer goes to sleep, the interface is set down, and
>> hwrng_unregister() is called. When the deadlock is hit, sleep is delayed
>> for tens of seconds, causing it to fail. These scenarios are:
>>
>> 1) The hwrng kthread can't be stopped while it's sleeping, because it
>> uses msleep_interruptible() instead of schedule_timeout_interruptible().
>> The fix is a simple moving to the correct function. At the same time,
>> we should cleanup a common and useless dmesg splat in the same area.
>>
>> 2) A normal user thread can't be interrupted by hwrng_unregister() while
>> it's sleeping, because hwrng_unregister() is called from elsewhere.
>> The solution here is to keep track of which thread is currently
>> reading, and asleep, and signal that thread when it's time to
>> unregister. There's a bit of book keeping required to prevent
>> lifetime issues on current.
>>
>> Reported-by: Gregory Erwin <gregerwin256@gmail.com>
>> Cc: Toke Høiland-Jørgensen <toke@redhat.com>
>> Cc: Kalle Valo <kvalo@kernel.org>
>> Cc: Rui Salvaterra <rsalvaterra@gmail.com>
>> Cc: Herbert Xu <herbert@gondor.apana.org.au>
>> Cc: stable@vger.kernel.org
>> Fixes: fcd09c90c3c5 ("ath9k: use hw_random API instead of directly dumping into random.c")
>> Link: https://lore.kernel.org/all/CAO+Okf6ZJC5-nTE_EJUGQtd8JiCkiEHytGgDsFGTEjs0c00giw@mail.gmail.com/
>> Link: https://lore.kernel.org/lkml/CAO+Okf5k+C+SE6pMVfPf-d8MfVPVq4PO7EY8Hys_DVXtent3HA@mail.gmail.com/
>> Link: https://bugs.archlinux.org/task/75138
>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>> ---
>> Changes v7->v8:
>> - Add a missing export_symbol.
>>
>> drivers/char/hw_random/core.c | 30 ++++++++++++++++++++++++----
>> drivers/net/wireless/ath/ath9k/rng.c | 19 +++++++-----------
>> kernel/sched/core.c | 1 +
>> 3 files changed, 34 insertions(+), 16 deletions(-)
>
> I don't see any acks for the hw_random and the scheduler change, adding more
> people to CC. Full patch here:
>
> https://patchwork.kernel.org/project/linux-wireless/patch/20220629114240.946411-1-Jason@zx2c4.com/
>
> Are everyone ok if I take this patch via wireless-next?
>
Thanks for the Cc.
I'm not hot on the export of wake_up_state(), IMO any wakeup with
!(state & TASK_NORMAL) should be reserved to kernel internals. Now, here
IIUC the problem is that the patch uses an inline invoking
wake_up_state(p, TASK_INTERRUPTIBLE)
so this isn't playing with any 'exotic' task state, thus it shouldn't
actually need the export.
I've been trying to figure out if this could work with just a
wake_up_process(), but the sleeping pattern here is not very conforming
(cf. 'wait loop' pattern in sched/core.c), AFAICT the signal is used to
circumvent that :/
Hi Valentin,
On 7/11/22, Valentin Schneider <vschneid@redhat.com> wrote:
> On 07/07/22 19:26, Kalle Valo wrote:
>> "Jason A. Donenfeld" <Jason@zx2c4.com> writes:
>>
>>> There are two deadlock scenarios that need addressing, which cause
>>> problems when the computer goes to sleep, the interface is set down, and
>>> hwrng_unregister() is called. When the deadlock is hit, sleep is delayed
>>> for tens of seconds, causing it to fail. These scenarios are:
>>>
>>> 1) The hwrng kthread can't be stopped while it's sleeping, because it
>>> uses msleep_interruptible() instead of
>>> schedule_timeout_interruptible().
>>> The fix is a simple moving to the correct function. At the same time,
>>> we should cleanup a common and useless dmesg splat in the same area.
>>>
>>> 2) A normal user thread can't be interrupted by hwrng_unregister() while
>>> it's sleeping, because hwrng_unregister() is called from elsewhere.
>>> The solution here is to keep track of which thread is currently
>>> reading, and asleep, and signal that thread when it's time to
>>> unregister. There's a bit of book keeping required to prevent
>>> lifetime issues on current.
>>>
>>> Reported-by: Gregory Erwin <gregerwin256@gmail.com>
>>> Cc: Toke Høiland-Jørgensen <toke@redhat.com>
>>> Cc: Kalle Valo <kvalo@kernel.org>
>>> Cc: Rui Salvaterra <rsalvaterra@gmail.com>
>>> Cc: Herbert Xu <herbert@gondor.apana.org.au>
>>> Cc: stable@vger.kernel.org
>>> Fixes: fcd09c90c3c5 ("ath9k: use hw_random API instead of directly
>>> dumping into random.c")
>>> Link:
>>> https://lore.kernel.org/all/CAO+Okf6ZJC5-nTE_EJUGQtd8JiCkiEHytGgDsFGTEjs0c00giw@mail.gmail.com/
>>> Link:
>>> https://lore.kernel.org/lkml/CAO+Okf5k+C+SE6pMVfPf-d8MfVPVq4PO7EY8Hys_DVXtent3HA@mail.gmail.com/
>>> Link: https://bugs.archlinux.org/task/75138
>>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>>> ---
>>> Changes v7->v8:
>>> - Add a missing export_symbol.
>>>
>>> drivers/char/hw_random/core.c | 30 ++++++++++++++++++++++++----
>>> drivers/net/wireless/ath/ath9k/rng.c | 19 +++++++-----------
>>> kernel/sched/core.c | 1 +
>>> 3 files changed, 34 insertions(+), 16 deletions(-)
>>
>> I don't see any acks for the hw_random and the scheduler change, adding
>> more
>> people to CC. Full patch here:
>>
>> https://patchwork.kernel.org/project/linux-wireless/patch/20220629114240.946411-1-Jason@zx2c4.com/
>>
>> Are everyone ok if I take this patch via wireless-next?
>>
>
> Thanks for the Cc.
>
> I'm not hot on the export of wake_up_state(), IMO any wakeup with
> !(state & TASK_NORMAL) should be reserved to kernel internals. Now, here
> IIUC the problem is that the patch uses an inline invoking
>
> wake_up_state(p, TASK_INTERRUPTIBLE)
>
> so this isn't playing with any 'exotic' task state, thus it shouldn't
> actually need the export.
>
> I've been trying to figure out if this could work with just a
> wake_up_process(), but the sleeping pattern here is not very conforming
> (cf. 'wait loop' pattern in sched/core.c), AFAICT the signal is used to
> circumvent that :/
I don't intend to work on this patch more. If you'd like to ack the
trivial scheduler change (adding EXPORT_SYMBOL), that'd help, and then
this can move forward as planned. Otherwise, if you have particular
opinions about this patch that you want to happen, feel free to pick
up the patch and send your own revisions (though I don't intend to do
further review). Alternatively, I'll just send a patch to remove the
driver entirely. Hopefully you do find this ack-able, though.
Jason
On 11/07/22 13:53, Jason A. Donenfeld wrote: > Hi Valentin, > > On 7/11/22, Valentin Schneider <vschneid@redhat.com> wrote: >> Thanks for the Cc. >> >> I'm not hot on the export of wake_up_state(), IMO any wakeup with >> !(state & TASK_NORMAL) should be reserved to kernel internals. Now, here >> IIUC the problem is that the patch uses an inline invoking >> >> wake_up_state(p, TASK_INTERRUPTIBLE) >> >> so this isn't playing with any 'exotic' task state, thus it shouldn't >> actually need the export. >> >> I've been trying to figure out if this could work with just a >> wake_up_process(), but the sleeping pattern here is not very conforming >> (cf. 'wait loop' pattern in sched/core.c), AFAICT the signal is used to >> circumvent that :/ > > I don't intend to work on this patch more. If you'd like to ack the > trivial scheduler change (adding EXPORT_SYMBOL), that'd help, and then > this can move forward as planned. Otherwise, if you have particular > opinions about this patch that you want to happen, feel free to pick > up the patch and send your own revisions (though I don't intend to do > further review). Alternatively, I'll just send a patch to remove the > driver entirely. Hopefully you do find this ack-able, though. > I'm not for a blanket wake_up_state() export, however if we *really* need it then I suppose we could have a wake_up_process_interruptible() exported and used by __set_notify_signal().
Hi Valentin, On Tue, Jul 19, 2022 at 04:15:02PM +0100, Valentin Schneider wrote: > On 11/07/22 13:53, Jason A. Donenfeld wrote: > > Hi Valentin, > > > > On 7/11/22, Valentin Schneider <vschneid@redhat.com> wrote: > >> Thanks for the Cc. > >> > >> I'm not hot on the export of wake_up_state(), IMO any wakeup with > >> !(state & TASK_NORMAL) should be reserved to kernel internals. Now, here > >> IIUC the problem is that the patch uses an inline invoking > >> > >> wake_up_state(p, TASK_INTERRUPTIBLE) > >> > >> so this isn't playing with any 'exotic' task state, thus it shouldn't > >> actually need the export. > >> > >> I've been trying to figure out if this could work with just a > >> wake_up_process(), but the sleeping pattern here is not very conforming > >> (cf. 'wait loop' pattern in sched/core.c), AFAICT the signal is used to > >> circumvent that :/ > > > > I don't intend to work on this patch more. If you'd like to ack the > > trivial scheduler change (adding EXPORT_SYMBOL), that'd help, and then > > this can move forward as planned. Otherwise, if you have particular > > opinions about this patch that you want to happen, feel free to pick > > up the patch and send your own revisions (though I don't intend to do > > further review). Alternatively, I'll just send a patch to remove the > > driver entirely. Hopefully you do find this ack-able, though. > > > > I'm not for a blanket wake_up_state() export, however if we *really* need > it then I suppose we could have a wake_up_process_interruptible() exported > and used by __set_notify_signal(). > Thanks for keeping this thread alive. I'll do what you suggest and send a v+1. I think I understand the idea. Let's see how it goes. Jason
"Jason A. Donenfeld" <Jason@zx2c4.com> writes:
> There are two deadlock scenarios that need addressing, which cause
> problems when the computer goes to sleep, the interface is set down, and
> hwrng_unregister() is called. When the deadlock is hit, sleep is delayed
> for tens of seconds, causing it to fail. These scenarios are:
>
> 1) The hwrng kthread can't be stopped while it's sleeping, because it
> uses msleep_interruptible() instead of schedule_timeout_interruptible().
> The fix is a simple moving to the correct function. At the same time,
> we should cleanup a common and useless dmesg splat in the same area.
>
> 2) A normal user thread can't be interrupted by hwrng_unregister() while
> it's sleeping, because hwrng_unregister() is called from elsewhere.
> The solution here is to keep track of which thread is currently
> reading, and asleep, and signal that thread when it's time to
> unregister. There's a bit of book keeping required to prevent
> lifetime issues on current.
>
> Reported-by: Gregory Erwin <gregerwin256@gmail.com>
> Cc: Toke Høiland-Jørgensen <toke@redhat.com>
> Cc: Kalle Valo <kvalo@kernel.org>
> Cc: Rui Salvaterra <rsalvaterra@gmail.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: stable@vger.kernel.org
> Fixes: fcd09c90c3c5 ("ath9k: use hw_random API instead of directly dumping into random.c")
> Link: https://lore.kernel.org/all/CAO+Okf6ZJC5-nTE_EJUGQtd8JiCkiEHytGgDsFGTEjs0c00giw@mail.gmail.com/
> Link: https://lore.kernel.org/lkml/CAO+Okf5k+C+SE6pMVfPf-d8MfVPVq4PO7EY8Hys_DVXtent3HA@mail.gmail.com/
> Link: https://bugs.archlinux.org/task/75138
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
With the change to EXPORT_SYMBOL_GPL() for wake_up_state that Kalle has
kindly agreed to fix up while applying:
Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
Hey Gregory,
On Wed, Jun 29, 2022 at 01:42:40PM +0200, Jason A. Donenfeld wrote:
> There are two deadlock scenarios that need addressing, which cause
> problems when the computer goes to sleep, the interface is set down, and
> hwrng_unregister() is called. When the deadlock is hit, sleep is delayed
> for tens of seconds, causing it to fail. These scenarios are:
>
> 1) The hwrng kthread can't be stopped while it's sleeping, because it
> uses msleep_interruptible() instead of schedule_timeout_interruptible().
> The fix is a simple moving to the correct function. At the same time,
> we should cleanup a common and useless dmesg splat in the same area.
>
> 2) A normal user thread can't be interrupted by hwrng_unregister() while
> it's sleeping, because hwrng_unregister() is called from elsewhere.
> The solution here is to keep track of which thread is currently
> reading, and asleep, and signal that thread when it's time to
> unregister. There's a bit of book keeping required to prevent
> lifetime issues on current.
>
> Reported-by: Gregory Erwin <gregerwin256@gmail.com>
> Cc: Toke Høiland-Jørgensen <toke@redhat.com>
> Cc: Kalle Valo <kvalo@kernel.org>
> Cc: Rui Salvaterra <rsalvaterra@gmail.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: stable@vger.kernel.org
> Fixes: fcd09c90c3c5 ("ath9k: use hw_random API instead of directly dumping into random.c")
> Link: https://lore.kernel.org/all/CAO+Okf6ZJC5-nTE_EJUGQtd8JiCkiEHytGgDsFGTEjs0c00giw@mail.gmail.com/
> Link: https://lore.kernel.org/lkml/CAO+Okf5k+C+SE6pMVfPf-d8MfVPVq4PO7EY8Hys_DVXtent3HA@mail.gmail.com/
> Link: https://bugs.archlinux.org/task/75138
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Hoping for your `Tested-by:` if this still works for you.
Jason
On Thu, Jun 30, 2022 at 7:03 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hey Gregory,
>
> On Wed, Jun 29, 2022 at 01:42:40PM +0200, Jason A. Donenfeld wrote:
> > There are two deadlock scenarios that need addressing, which cause
> > problems when the computer goes to sleep, the interface is set down, and
> > hwrng_unregister() is called. When the deadlock is hit, sleep is delayed
> > for tens of seconds, causing it to fail. These scenarios are:
> >
> > 1) The hwrng kthread can't be stopped while it's sleeping, because it
> > uses msleep_interruptible() instead of schedule_timeout_interruptible().
> > The fix is a simple moving to the correct function. At the same time,
> > we should cleanup a common and useless dmesg splat in the same area.
> >
> > 2) A normal user thread can't be interrupted by hwrng_unregister() while
> > it's sleeping, because hwrng_unregister() is called from elsewhere.
> > The solution here is to keep track of which thread is currently
> > reading, and asleep, and signal that thread when it's time to
> > unregister. There's a bit of book keeping required to prevent
> > lifetime issues on current.
> >
> > Reported-by: Gregory Erwin <gregerwin256@gmail.com>
> > Cc: Toke Høiland-Jørgensen <toke@redhat.com>
> > Cc: Kalle Valo <kvalo@kernel.org>
> > Cc: Rui Salvaterra <rsalvaterra@gmail.com>
> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
> > Cc: stable@vger.kernel.org
> > Fixes: fcd09c90c3c5 ("ath9k: use hw_random API instead of directly dumping into random.c")
> > Link: https://lore.kernel.org/all/CAO+Okf6ZJC5-nTE_EJUGQtd8JiCkiEHytGgDsFGTEjs0c00giw@mail.gmail.com/
> > Link: https://lore.kernel.org/lkml/CAO+Okf5k+C+SE6pMVfPf-d8MfVPVq4PO7EY8Hys_DVXtent3HA@mail.gmail.com/
> > Link: https://bugs.archlinux.org/task/75138
> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>
> Hoping for your `Tested-by:` if this still works for you.
>
> Jason
Apologies for the delay.
Tested-by: Gregory Erwin <gregerwin256@gmail.com>
On Wed, Jun 29, 2022 at 01:42:40PM +0200, Jason A. Donenfeld wrote:
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -4284,6 +4284,7 @@ int wake_up_state(struct task_struct *p, unsigned int state)
> {
> return try_to_wake_up(p, state, 0);
> }
> +EXPORT_SYMBOL(wake_up_state);
Should be EXPORT_SYMBOL_GPL(), right?
thanks,
greg k-h
On Wed, Jun 29, 2022 at 5:28 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jun 29, 2022 at 01:42:40PM +0200, Jason A. Donenfeld wrote:
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -4284,6 +4284,7 @@ int wake_up_state(struct task_struct *p, unsigned int state)
> > {
> > return try_to_wake_up(p, state, 0);
> > }
> > +EXPORT_SYMBOL(wake_up_state);
>
> Should be EXPORT_SYMBOL_GPL(), right?
The highly similar wake_up_process() above it, which has the exact
same body, except the `state` argument is fixed as TASK_NORMAL, is an
EXPORT_SYMBOL(). So I figured this one should follow form. Let me know
if that's silly, and I'll send a v+1 changing it to _GPL though.
Jason
On Wed, Jun 29, 2022 at 06:15:49PM +0200, Jason A. Donenfeld wrote:
> On Wed, Jun 29, 2022 at 5:28 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Jun 29, 2022 at 01:42:40PM +0200, Jason A. Donenfeld wrote:
> > > --- a/kernel/sched/core.c
> > > +++ b/kernel/sched/core.c
> > > @@ -4284,6 +4284,7 @@ int wake_up_state(struct task_struct *p, unsigned int state)
> > > {
> > > return try_to_wake_up(p, state, 0);
> > > }
> > > +EXPORT_SYMBOL(wake_up_state);
> >
> > Should be EXPORT_SYMBOL_GPL(), right?
>
> The highly similar wake_up_process() above it, which has the exact
> same body, except the `state` argument is fixed as TASK_NORMAL, is an
> EXPORT_SYMBOL(). So I figured this one should follow form. Let me know
> if that's silly, and I'll send a v+1 changing it to _GPL though.
I'll let the maintainers of this code decide that, I wasn't aware of the
other symbol above this. It's their call, as it's their code.
thanks,
greg k-h
© 2016 - 2026 Red Hat, Inc.