* [bug report] net/smc: CLC accept / confirm V2
@ 2020-10-28 11:25 Dan Carpenter
2020-10-28 13:23 ` Ursula Braun
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2020-10-28 11:25 UTC (permalink / raw)
To: ubraun; +Cc: linux-s390
Hello Ursula Braun,
The patch a7c9c5f4af7f: "net/smc: CLC accept / confirm V2" from Sep
26, 2020, leads to the following static checker warning:
net/smc/af_smc.c:1771 smc_listen_work()
error: we previously assumed 'ini' could be null (see line 1715)
net/smc/af_smc.c
1665 static void smc_listen_work(struct work_struct *work)
1666 {
1667 struct smc_sock *new_smc = container_of(work, struct smc_sock,
1668 smc_listen_work);
1669 u8 version = smc_ism_v2_capable ? SMC_V2 : SMC_V1;
1670 struct socket *newclcsock = new_smc->clcsock;
1671 struct smc_clc_msg_accept_confirm *cclc;
1672 struct smc_clc_msg_proposal_area *buf;
1673 struct smc_clc_msg_proposal *pclc;
1674 struct smc_init_info *ini = NULL;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1675 int rc = 0;
1676
1677 if (new_smc->listen_smc->sk.sk_state != SMC_LISTEN)
1678 return smc_listen_out_err(new_smc);
1679
1680 if (new_smc->use_fallback) {
1681 smc_listen_out_connected(new_smc);
1682 return;
1683 }
1684
1685 /* check if peer is smc capable */
1686 if (!tcp_sk(newclcsock->sk)->syn_smc) {
1687 smc_switch_to_fallback(new_smc);
1688 new_smc->fallback_rsn = SMC_CLC_DECL_PEERNOSMC;
1689 smc_listen_out_connected(new_smc);
1690 return;
1691 }
1692
1693 /* do inband token exchange -
1694 * wait for and receive SMC Proposal CLC message
1695 */
1696 buf = kzalloc(sizeof(*buf), GFP_KERNEL);
1697 if (!buf) {
1698 rc = SMC_CLC_DECL_MEM;
1699 goto out_decl;
^^^^^^^^^^^^^^
There are several paths where "ini" is NULL leading to an Oops in the
error handling.
1700 }
1701 pclc = (struct smc_clc_msg_proposal *)buf;
1702 rc = smc_clc_wait_msg(new_smc, pclc, sizeof(*buf),
1703 SMC_CLC_PROPOSAL, CLC_WAIT_TIME);
1704 if (rc)
1705 goto out_decl;
^^^^^^^^^^^^^
1706 version = pclc->hdr.version == SMC_V1 ? SMC_V1 : version;
1707
1708 /* IPSec connections opt out of SMC optimizations */
1709 if (using_ipsec(new_smc)) {
1710 rc = SMC_CLC_DECL_IPSEC;
1711 goto out_decl;
^^^^^^^^^^^^^
1712 }
1713
1714 ini = kzalloc(sizeof(*ini), GFP_KERNEL);
1715 if (!ini) {
1716 rc = SMC_CLC_DECL_MEM;
1717 goto out_decl;
^^^^^^^^^^^^^
1718 }
1719
1720 /* initial version checking */
1721 rc = smc_listen_v2_check(new_smc, pclc, ini);
1722 if (rc)
1723 goto out_decl;
1724
1725 mutex_lock(&smc_server_lgr_pending);
1726 smc_close_init(new_smc);
1727 smc_rx_init(new_smc);
1728 smc_tx_init(new_smc);
1729
1730 /* determine ISM or RoCE device used for connection */
1731 rc = smc_listen_find_device(new_smc, pclc, ini);
1732 if (rc)
1733 goto out_unlock;
1734
1735 /* send SMC Accept CLC message */
1736 rc = smc_clc_send_accept(new_smc, ini->first_contact_local,
1737 ini->smcd_version == SMC_V2 ? SMC_V2 : SMC_V1);
1738 if (rc)
1739 goto out_unlock;
1740
1741 /* SMC-D does not need this lock any more */
1742 if (ini->is_smcd)
1743 mutex_unlock(&smc_server_lgr_pending);
1744
1745 /* receive SMC Confirm CLC message */
1746 memset(buf, 0, sizeof(*buf));
1747 cclc = (struct smc_clc_msg_accept_confirm *)buf;
1748 rc = smc_clc_wait_msg(new_smc, cclc, sizeof(*buf),
1749 SMC_CLC_CONFIRM, CLC_WAIT_TIME);
1750 if (rc) {
1751 if (!ini->is_smcd)
1752 goto out_unlock;
1753 goto out_decl;
1754 }
1755
1756 /* finish worker */
1757 if (!ini->is_smcd) {
1758 rc = smc_listen_rdma_finish(new_smc, cclc,
1759 ini->first_contact_local);
1760 if (rc)
1761 goto out_unlock;
1762 mutex_unlock(&smc_server_lgr_pending);
1763 }
1764 smc_conn_save_peer_info(new_smc, cclc);
1765 smc_listen_out_connected(new_smc);
1766 goto out_free;
1767
1768 out_unlock:
1769 mutex_unlock(&smc_server_lgr_pending);
1770 out_decl:
1771 smc_listen_decline(new_smc, rc, ini, version);
^^^
If "ini" is NULL then this will Oops.
1772 out_free:
1773 kfree(ini);
1774 kfree(buf);
1775 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] net/smc: CLC accept / confirm V2
2020-10-28 11:25 [bug report] net/smc: CLC accept / confirm V2 Dan Carpenter
@ 2020-10-28 13:23 ` Ursula Braun
0 siblings, 0 replies; 2+ messages in thread
From: Ursula Braun @ 2020-10-28 13:23 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-s390, Karsten Graul
On 10/28/20 12:25 PM, Dan Carpenter wrote:
> Hello Ursula Braun,
>
> The patch a7c9c5f4af7f: "net/smc: CLC accept / confirm V2" from Sep
> 26, 2020, leads to the following static checker warning:
>
> net/smc/af_smc.c:1771 smc_listen_work()
> error: we previously assumed 'ini' could be null (see line 1715)
>
Thanks for reporting this problem!
Karsten Graul provided already this fix on 10/23/20 (accepted by
Jakub Kicinski 10/27/20) for the net-tree:
net/smc: fix null pointer dereference in smc_listen_decline()
smc_listen_work() calls smc_listen_decline() on label out_decl,
providing the ini pointer variable. But this pointer can still be null
when the label out_decl is reached.
Fix this by checking the ini variable in smc_listen_work() and call
smc_listen_decline() with the result directly.
Fixes: a7c9c5f4af7f ("net/smc: CLC accept / confirm V2")
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
This fix is supposed to solve your reported problem.
Regards, Ursula Braun
> net/smc/af_smc.c
> 1665 static void smc_listen_work(struct work_struct *work)
> 1666 {
> 1667 struct smc_sock *new_smc = container_of(work, struct smc_sock,
> 1668 smc_listen_work);
> 1669 u8 version = smc_ism_v2_capable ? SMC_V2 : SMC_V1;
> 1670 struct socket *newclcsock = new_smc->clcsock;
> 1671 struct smc_clc_msg_accept_confirm *cclc;
> 1672 struct smc_clc_msg_proposal_area *buf;
> 1673 struct smc_clc_msg_proposal *pclc;
> 1674 struct smc_init_info *ini = NULL;
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> 1675 int rc = 0;
> 1676
> 1677 if (new_smc->listen_smc->sk.sk_state != SMC_LISTEN)
> 1678 return smc_listen_out_err(new_smc);
> 1679
> 1680 if (new_smc->use_fallback) {
> 1681 smc_listen_out_connected(new_smc);
> 1682 return;
> 1683 }
> 1684
> 1685 /* check if peer is smc capable */
> 1686 if (!tcp_sk(newclcsock->sk)->syn_smc) {
> 1687 smc_switch_to_fallback(new_smc);
> 1688 new_smc->fallback_rsn = SMC_CLC_DECL_PEERNOSMC;
> 1689 smc_listen_out_connected(new_smc);
> 1690 return;
> 1691 }
> 1692
> 1693 /* do inband token exchange -
> 1694 * wait for and receive SMC Proposal CLC message
> 1695 */
> 1696 buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> 1697 if (!buf) {
> 1698 rc = SMC_CLC_DECL_MEM;
> 1699 goto out_decl;
> ^^^^^^^^^^^^^^
> There are several paths where "ini" is NULL leading to an Oops in the
> error handling.
>
> 1700 }
> 1701 pclc = (struct smc_clc_msg_proposal *)buf;
> 1702 rc = smc_clc_wait_msg(new_smc, pclc, sizeof(*buf),
> 1703 SMC_CLC_PROPOSAL, CLC_WAIT_TIME);
> 1704 if (rc)
> 1705 goto out_decl;
> ^^^^^^^^^^^^^
>
> 1706 version = pclc->hdr.version == SMC_V1 ? SMC_V1 : version;
> 1707
> 1708 /* IPSec connections opt out of SMC optimizations */
> 1709 if (using_ipsec(new_smc)) {
> 1710 rc = SMC_CLC_DECL_IPSEC;
> 1711 goto out_decl;
> ^^^^^^^^^^^^^
>
> 1712 }
> 1713
> 1714 ini = kzalloc(sizeof(*ini), GFP_KERNEL);
> 1715 if (!ini) {
> 1716 rc = SMC_CLC_DECL_MEM;
> 1717 goto out_decl;
> ^^^^^^^^^^^^^
>
> 1718 }
> 1719
> 1720 /* initial version checking */
> 1721 rc = smc_listen_v2_check(new_smc, pclc, ini);
> 1722 if (rc)
> 1723 goto out_decl;
> 1724
> 1725 mutex_lock(&smc_server_lgr_pending);
> 1726 smc_close_init(new_smc);
> 1727 smc_rx_init(new_smc);
> 1728 smc_tx_init(new_smc);
> 1729
> 1730 /* determine ISM or RoCE device used for connection */
> 1731 rc = smc_listen_find_device(new_smc, pclc, ini);
> 1732 if (rc)
> 1733 goto out_unlock;
> 1734
> 1735 /* send SMC Accept CLC message */
> 1736 rc = smc_clc_send_accept(new_smc, ini->first_contact_local,
> 1737 ini->smcd_version == SMC_V2 ? SMC_V2 : SMC_V1);
> 1738 if (rc)
> 1739 goto out_unlock;
> 1740
> 1741 /* SMC-D does not need this lock any more */
> 1742 if (ini->is_smcd)
> 1743 mutex_unlock(&smc_server_lgr_pending);
> 1744
> 1745 /* receive SMC Confirm CLC message */
> 1746 memset(buf, 0, sizeof(*buf));
> 1747 cclc = (struct smc_clc_msg_accept_confirm *)buf;
> 1748 rc = smc_clc_wait_msg(new_smc, cclc, sizeof(*buf),
> 1749 SMC_CLC_CONFIRM, CLC_WAIT_TIME);
> 1750 if (rc) {
> 1751 if (!ini->is_smcd)
> 1752 goto out_unlock;
> 1753 goto out_decl;
> 1754 }
> 1755
> 1756 /* finish worker */
> 1757 if (!ini->is_smcd) {
> 1758 rc = smc_listen_rdma_finish(new_smc, cclc,
> 1759 ini->first_contact_local);
> 1760 if (rc)
> 1761 goto out_unlock;
> 1762 mutex_unlock(&smc_server_lgr_pending);
> 1763 }
> 1764 smc_conn_save_peer_info(new_smc, cclc);
> 1765 smc_listen_out_connected(new_smc);
> 1766 goto out_free;
> 1767
> 1768 out_unlock:
> 1769 mutex_unlock(&smc_server_lgr_pending);
> 1770 out_decl:
> 1771 smc_listen_decline(new_smc, rc, ini, version);
> ^^^
> If "ini" is NULL then this will Oops.
>
> 1772 out_free:
> 1773 kfree(ini);
> 1774 kfree(buf);
> 1775 }
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-10-28 22:03 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-28 11:25 [bug report] net/smc: CLC accept / confirm V2 Dan Carpenter
2020-10-28 13:23 ` Ursula Braun
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox