netdev - Re: [PATCH net] net/smc: fix potential sleeping issue in smc_switch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f614e5fe-29cb-42f5-a02b-b777c043e014@linux.ibm.com>
Date: Wed, 17 Apr 2024 17:17:52 +0200
From: Wenjia Zhang <wenjia@...ux.ibm.com>
To: Guangguan Wang <guangguan.wang@...ux.alibaba.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Zhengchao Shao <shaozhengchao@...wei.com>
Cc: jaka@...ux.ibm.com, alibuda@...ux.alibaba.com, tonylu@...ux.alibaba.com,
        guwen@...ux.alibaba.com, weiyongjun1@...wei.com, yuehaibing@...wei.com,
        tangchengchang@...wei.com, kuba@...nel.org, edumazet@...gle.com,
        davem@...emloft.net, netdev@...r.kernel.org,
        linux-s390@...r.kernel.org
Subject: Re: [PATCH net] net/smc: fix potential sleeping issue in
 smc_switch_conns



On 17.04.24 09:32, Guangguan Wang wrote:
> 
> 
> On 2024/4/16 20:06, Paolo Abeni wrote:
>> On Sat, 2024-04-13 at 11:51 +0800, Zhengchao Shao wrote:
>>> Potential sleeping issue exists in the following processes:
>>> smc_switch_conns
>>>    spin_lock_bh(&conn->send_lock)
>>>    smc_switch_link_and_count
>>>      smcr_link_put
>>>        __smcr_link_clear
>>>          smc_lgr_put
>>>            __smc_lgr_free
>>>              smc_lgr_free_bufs
>>>                __smc_lgr_free_bufs
>>>                  smc_buf_free
>>>                    smcr_buf_free
>>>                      smcr_buf_unmap_link
>>>                        smc_ib_put_memory_region
>>>                          ib_dereg_mr
>>>                            ib_dereg_mr_user
>>>                              mr->device->ops.dereg_mr
>>> If scheduling exists when the IB driver implements .dereg_mr hook
>>> function, the bug "scheduling while atomic" will occur. For example,
>>> cxgb4 and efa driver. Use mutex lock instead of spin lock to fix it.
>>
>> I tried to inspect all the lock call sites, and it *look* like they are
>> all in process context, so the switch should be feasible.
> 
> There exist some calls from tasklet, where mutex lock is infeasible.
> For example:
> - tasklet -> smc_wr_tx_tasklet_fn -> smc_wr_tx_process_cqe -> pnd_snd.handler -> smc_cdc_tx_handler -> smc_tx_pending -> smc_tx_sndbuf_nonempty -> smcr_tx_sndbuf_nonempty -> spin_lock_bh(&conn->send_lock)
> - tasklet -> smc_wr_rx_tasklet_fn -> smc_wr_rx_process_cqes -> smc_wr_rx_demultiplex -> smc_cdc_rx_handler -> smc_cdc_msg_validate -> spin_lock_bh(&conn->send_lock)
> 
> Thanks,
> Guangguan Wang
> Agree! Thank you, Guangguan, for the examples!
If we can verify that this bug exits, we should find other solutions.
>>
>> Still the fact that the existing lock is a BH variant is suspect.
>> Either the BH part was not needed or this can introduce subtle
>> regressions/issues.
>>
>> I think this deserves at least a 3rd party testing.
>>
>> Thanks,
>>
>> Paolo
>>
>