Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

__rust_probestack test in rust_example.rs is causing a stack overflow #111

Open
wedsonaf opened this issue Mar 15, 2021 · 5 comments
Open

__rust_probestack test in rust_example.rs is causing a stack overflow #111

wedsonaf opened this issue Mar 15, 2021 · 5 comments
Labels
• lib Related to the `rust/` library.

Comments

@wedsonaf
Copy link

The stack size is 8KB, having a local variable of 8KB+32 bytes takes it over the edge.

I have seen failures on debug builds for this, for example, https://github.com/Rust-for-Linux/linux/runs/2107043283?check_suite_focus=true -- it has the following error:

[   13.127113] Large array has length: 1028
[   13.131438] BUG: stack guard page was hit at (____ptrval____) (stack is (____ptrval____)..(____ptrval____))
[   13.131438] kernel stack overflow (double-fault): 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI

For now I'm reducing the variable size in #104 to work around this. Please fix if you need it to be bigger to ensure probestack is properly tested.
@alex
Copy link
Member

alex commented Mar 15, 2021

Maybe we should put it in a different test file so that it doesn't disrupt code that's expected to work :-)

@ojeda
Copy link
Member

ojeda commented Mar 15, 2021 •
edited
Loading

Indeed. Reorganizing all the test code is something I will work on, but first I need to finish setting up the "independent CI".

Nevertheless, it is good we are seeing the error in the debug builds, it means debug CI is being useful ;-)

@ojeda ojeda added upstream - optional • lib Related to the `rust/` library. labels Mar 15, 2021
@adamrk adamrk mentioned this issue Mar 16, 2021
Merged
@adamrk
Copy link

adamrk commented Mar 16, 2021

So this was triggered by an upstream merge from #112. Does anyone know why (just trying to understand myself)?

@ojeda
Copy link
Member

ojeda commented Mar 16, 2021

The CI link mentioned by Wedson happened before #112 -- it is not a particular change: the frame size has been growing with the added test cases, which means we are crossing the tipping point now.

Eventually we will move these into proper tests, but for the moment my plan is moving them into individual kernel modules since anyhow I need to clean up all the GitHub-only bits (which include rust_example.rs) to be able to get into linux-next properly.

@adamrk
Copy link

adamrk commented Mar 16, 2021

Got it, thanks!

@ojeda ojeda changed the title __rust_probestack test in rust_example.rs is causing a stack overflow __rust_probestack test in rust_example.rs is causing a stack overflow Mar 18, 2021
@TheSven73 TheSven73 mentioned this issue Apr 16, 2021
Merged
ojeda pushed a commit that referenced this issue Jan 10, 2022
@davem330
net/smc: fix kernel panic caused by race of smc_sock
349d431 
A crash occurs when smc_cdc_tx_handler() tries to access smc_sock
but smc_release() has already freed it.

[ 4570.695099] BUG: unable to handle page fault for address: 000000002eae9e88
[ 4570.696048] #PF: supervisor write access in kernel mode
[ 4570.696728] #PF: error_code(0x0002) - not-present page
[ 4570.697401] PGD 0 P4D 0
[ 4570.697716] Oops: 0002 [#1] PREEMPT SMP NOPTI
[ 4570.698228] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.16.0-rc4+ #111
[ 4570.699013] Hardware name: Alibaba Cloud Alibaba Cloud ECS, BIOS 8c24b4c 04/0
[ 4570.699933] RIP: 0010:_raw_spin_lock+0x1a/0x30
<...>
[ 4570.711446] Call Trace:
[ 4570.711746]  <IRQ>
[ 4570.711992]  smc_cdc_tx_handler+0x41/0xc0
[ 4570.712470]  smc_wr_tx_tasklet_fn+0x213/0x560
[ 4570.712981]  ? smc_cdc_tx_dismisser+0x10/0x10
[ 4570.713489]  tasklet_action_common.isra.17+0x66/0x140
[ 4570.714083]  __do_softirq+0x123/0x2f4
[ 4570.714521]  irq_exit_rcu+0xc4/0xf0
[ 4570.714934]  common_interrupt+0xba/0xe0

Though smc_cdc_tx_handler() checked the existence of smc connection,
smc_release() may have already dismissed and released the smc socket
before smc_cdc_tx_handler() further visits it.

smc_cdc_tx_handler()           |smc_release()
if (!conn)                     |
                               |
                               |smc_cdc_tx_dismiss_slots()
                               |      smc_cdc_tx_dismisser()
                               |
                               |sock_put(&smc->sk) <- last sock_put,
                               |                      smc_sock freed
bh_lock_sock(&smc->sk) (panic) |

To make sure we won't receive any CDC messages after we free the
smc_sock, add a refcount on the smc_connection for inflight CDC
message(posted to the QP but haven't received related CQE), and
don't release the smc_connection until all the inflight CDC messages
haven been done, for both success or failed ones.

Using refcount on CDC messages brings another problem: when the link
is going to be destroyed, smcr_link_clear() will reset the QP, which
then remove all the pending CQEs related to the QP in the CQ. To make
sure all the CQEs will always come back so the refcount on the
smc_connection can always reach 0, smc_ib_modify_qp_reset() was replaced
by smc_ib_modify_qp_error().
And remove the timeout in smc_wr_tx_wait_no_pending_sends() since we
need to wait for all pending WQEs done, or we may encounter use-after-
free when handling CQEs.

For IB device removal routine, we need to wait for all the QPs on that
device been destroyed before we can destroy CQs on the device, or
the refcount on smc_connection won't reach 0 and smc_sock cannot be
released.

Fixes: 5f08318 ("smc: connection data control (CDC)")
Reported-by: Wen Gu <guwen@linux.alibaba.com>
Signed-off-by: Dust Li <dust.li@linux.alibaba.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
• lib Related to the `rust/` library.
Milestone
No milestone
Development

No branches or pull requests
4 participants
@alex @ojeda @wedsonaf @adamrk