From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: Guoyong Wang <guoyong.wang@mediatek.com>
Cc: Theodore Ts'o <tytso@mit.edu>, Tejun Heo <tj@kernel.org>,
Lai Jiangshan <jiangshanlai@gmail.com>,
Matthias Brugger <matthias.bgg@gmail.com>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com>,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, wsd_upstream@mediatek.com
Subject: Re: [PATCH] random: Fix the issue of '_might_sleep' function running in an atomic contex
Date: Mon, 18 Mar 2024 21:00:42 +0100 [thread overview]
Message-ID: <Zfidal8CEZStp3R7@zx2c4.com> (raw)
In-Reply-To: <20240318075327.26318-1-guoyong.wang@mediatek.com>
Hi Guoyong,
On Mon, Mar 18, 2024 at 03:53:27PM +0800, Guoyong Wang wrote:
> 'input_handle_event' runs in an atomic context
> (spinlock). In rare instances, it may call
> the '_might_sleep' function, which could trigger
> a kernel exception.
>
> Backtrace:
> [<ffffffd613025ba0>] die+0xa8/0x2fc
> [<ffffffd613027428>] bug_handler+0x44/0xec
> [<ffffffd613016964>] brk_handler+0x90/0x144
> [<ffffffd613041e58>] do_debug_exception+0xa0/0x148
> [<ffffffd61400c208>] el1_dbg+0x60/0x7c
> [<ffffffd61400c000>] el1h_64_sync_handler+0x38/0x90
> [<ffffffd613011294>] el1h_64_sync+0x64/0x6c
> [<ffffffd613102d88>] __might_resched+0x1fc/0x2e8
> [<ffffffd613102b54>] __might_sleep+0x44/0x7c
> [<ffffffd6130b6eac>] cpus_read_lock+0x1c/0xec
> [<ffffffd6132c2820>] static_key_enable+0x14/0x38
> [<ffffffd61400ac08>] crng_set_ready+0x14/0x28
> [<ffffffd6130df4dc>] execute_in_process_context+0xb8/0xf8
> [<ffffffd61400ab30>] _credit_init_bits+0x118/0x1dc
> [<ffffffd6138580c8>] add_timer_randomness+0x264/0x270
> [<ffffffd613857e54>] add_input_randomness+0x38/0x48
> [<ffffffd613a80f94>] input_handle_event+0x2b8/0x490
> [<ffffffd613a81310>] input_event+0x6c/0x98
Thanks for reporting this.
I'm wondering, though, rather than introducing a second function, maybe
execute_in_process_context() should just gain a `&& !in_atomic()`.
That'd make things a bit simpler.
However, I'm pretty sure in_atomic() isn't actually a reliable way of
determining that, depending on config. So maybe this should just call
the worker always (if system_wq isn't null).
Alternatively, any chance the call to add_input_randomness() could be
moved outside the spinlock, or does this not look possible?
Jason
WARNING: multiple messages have this Message-ID (diff)
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: Guoyong Wang <guoyong.wang@mediatek.com>
Cc: Theodore Ts'o <tytso@mit.edu>, Tejun Heo <tj@kernel.org>,
Lai Jiangshan <jiangshanlai@gmail.com>,
Matthias Brugger <matthias.bgg@gmail.com>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com>,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, wsd_upstream@mediatek.com
Subject: Re: [PATCH] random: Fix the issue of '_might_sleep' function running in an atomic contex
Date: Mon, 18 Mar 2024 21:00:42 +0100 [thread overview]
Message-ID: <Zfidal8CEZStp3R7@zx2c4.com> (raw)
In-Reply-To: <20240318075327.26318-1-guoyong.wang@mediatek.com>
Hi Guoyong,
On Mon, Mar 18, 2024 at 03:53:27PM +0800, Guoyong Wang wrote:
> 'input_handle_event' runs in an atomic context
> (spinlock). In rare instances, it may call
> the '_might_sleep' function, which could trigger
> a kernel exception.
>
> Backtrace:
> [<ffffffd613025ba0>] die+0xa8/0x2fc
> [<ffffffd613027428>] bug_handler+0x44/0xec
> [<ffffffd613016964>] brk_handler+0x90/0x144
> [<ffffffd613041e58>] do_debug_exception+0xa0/0x148
> [<ffffffd61400c208>] el1_dbg+0x60/0x7c
> [<ffffffd61400c000>] el1h_64_sync_handler+0x38/0x90
> [<ffffffd613011294>] el1h_64_sync+0x64/0x6c
> [<ffffffd613102d88>] __might_resched+0x1fc/0x2e8
> [<ffffffd613102b54>] __might_sleep+0x44/0x7c
> [<ffffffd6130b6eac>] cpus_read_lock+0x1c/0xec
> [<ffffffd6132c2820>] static_key_enable+0x14/0x38
> [<ffffffd61400ac08>] crng_set_ready+0x14/0x28
> [<ffffffd6130df4dc>] execute_in_process_context+0xb8/0xf8
> [<ffffffd61400ab30>] _credit_init_bits+0x118/0x1dc
> [<ffffffd6138580c8>] add_timer_randomness+0x264/0x270
> [<ffffffd613857e54>] add_input_randomness+0x38/0x48
> [<ffffffd613a80f94>] input_handle_event+0x2b8/0x490
> [<ffffffd613a81310>] input_event+0x6c/0x98
Thanks for reporting this.
I'm wondering, though, rather than introducing a second function, maybe
execute_in_process_context() should just gain a `&& !in_atomic()`.
That'd make things a bit simpler.
However, I'm pretty sure in_atomic() isn't actually a reliable way of
determining that, depending on config. So maybe this should just call
the worker always (if system_wq isn't null).
Alternatively, any chance the call to add_input_randomness() could be
moved outside the spinlock, or does this not look possible?
Jason
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2024-03-18 20:00 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-18 7:53 [PATCH] random: Fix the issue of '_might_sleep' function running in an atomic contex Guoyong Wang
2024-03-18 7:53 ` Guoyong Wang
2024-03-18 20:00 ` Jason A. Donenfeld [this message]
2024-03-18 20:00 ` Jason A. Donenfeld
2024-03-19 9:30 ` Guoyong Wang
2024-03-19 9:30 ` Guoyong Wang
2024-03-20 1:09 ` Jason A. Donenfeld
2024-03-20 1:09 ` Jason A. Donenfeld
2024-03-20 9:02 ` Guoyong Wang
2024-03-20 9:02 ` Guoyong Wang
2024-04-02 8:12 ` Guoyong Wang
2024-04-02 8:12 ` Guoyong Wang
2024-04-17 12:01 ` [PATCH] random: handle creditable entropy from atomic process context Jason A. Donenfeld
2024-04-17 12:01 ` Jason A. Donenfeld
2024-04-19 8:41 ` [PATCH] random: Fix the issue of '_might_sleep' function running in an atomic contex Guoyong Wang
2024-04-19 8:41 ` Guoyong Wang
2024-04-19 8:55 ` Jason A. Donenfeld
2024-04-19 8:55 ` Jason A. Donenfeld
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zfidal8CEZStp3R7@zx2c4.com \
--to=jason@zx2c4.com \
--cc=angelogioacchino.delregno@collabora.com \
--cc=guoyong.wang@mediatek.com \
--cc=jiangshanlai@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
--cc=tj@kernel.org \
--cc=tytso@mit.edu \
--cc=wsd_upstream@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.