* [PATCH net 1/4] net/smc: fix toleration of fake add_link messages
2020-09-03 19:53 [PATCH net 0/4] net/smc: fixes 2020-09-03 Karsten Graul
@ 2020-09-03 19:53 ` Karsten Graul
2020-09-03 19:53 ` [PATCH net 2/4] net/smc: set rx_off for SMCR explicitly Karsten Graul
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Karsten Graul @ 2020-09-03 19:53 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, heiko.carstens, raspl, ubraun
Older SMCR implementations had no link failover support and used one
link only. Because the handshake protocol requires to try the
establishment of a second link the old code sent a fake add_link message
and declined any server response afterwards.
The current code supports multiple links and inspects the received fake
add_link message more closely. To tolerate the fake add_link messages
smc_llc_is_local_add_link() needs an improved check of the message to
be able to separate between locally enqueued and fake add_link messages.
And smc_llc_cli_add_link() needs to check if the provided qp_mtu size is
invalid and reject the add_link request in that case.
Fixes: c48254fa48e5 ("net/smc: move add link processing for new device into llc layer")
Reviewed-by: Ursula Braun <ubraun@linux.ibm.com>
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
---
net/smc/smc_llc.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index df5b0a6ea848..3ea33466ebe9 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -841,6 +841,9 @@ int smc_llc_cli_add_link(struct smc_link *link, struct smc_llc_qentry *qentry)
struct smc_init_info ini;
int lnk_idx, rc = 0;
+ if (!llc->qp_mtu)
+ goto out_reject;
+
ini.vlan_id = lgr->vlan_id;
smc_pnet_find_alt_roce(lgr, &ini, link->smcibdev);
if (!memcmp(llc->sender_gid, link->peer_gid, SMC_GID_SIZE) &&
@@ -917,10 +920,20 @@ static void smc_llc_cli_add_link_invite(struct smc_link *link,
kfree(qentry);
}
+static bool smc_llc_is_empty_llc_message(union smc_llc_msg *llc)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(llc->raw.data); i++)
+ if (llc->raw.data[i])
+ return false;
+ return true;
+}
+
static bool smc_llc_is_local_add_link(union smc_llc_msg *llc)
{
if (llc->raw.hdr.common.type == SMC_LLC_ADD_LINK &&
- !llc->add_link.qp_mtu && !llc->add_link.link_num)
+ smc_llc_is_empty_llc_message(llc))
return true;
return false;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net 2/4] net/smc: set rx_off for SMCR explicitly
2020-09-03 19:53 [PATCH net 0/4] net/smc: fixes 2020-09-03 Karsten Graul
2020-09-03 19:53 ` [PATCH net 1/4] net/smc: fix toleration of fake add_link messages Karsten Graul
@ 2020-09-03 19:53 ` Karsten Graul
2020-09-03 19:53 ` [PATCH net 3/4] net/smc: reset sndbuf_desc if freed Karsten Graul
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Karsten Graul @ 2020-09-03 19:53 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, heiko.carstens, raspl, ubraun
From: Ursula Braun <ubraun@linux.ibm.com>
SMC tries to make use of SMCD first. If a problem shows up,
it tries to switch to SMCR. If the SMCD initializing problem shows
up after the SMCD connection has already been initialized, field
rx_off keeps the wrong SMCD value for SMCR, which results in corrupted
data at the receiver.
This patch adds an explicit (re-)setting of field rx_off to zero if the
connection uses SMCR.
Fixes: be244f28d22f ("net/smc: add SMC-D support in data transfer")
Reviewed-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
---
net/smc/smc_core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index b42fa3b00d00..a6ac0eb20c99 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -1356,6 +1356,8 @@ int smc_conn_create(struct smc_sock *smc, struct smc_init_info *ini)
if (ini->is_smcd) {
conn->rx_off = sizeof(struct smcd_cdc_msg);
smcd_cdc_rx_init(conn); /* init tasklet for this conn */
+ } else {
+ conn->rx_off = 0;
}
#ifndef KERNEL_HAS_ATOMIC64
spin_lock_init(&conn->acurs_lock);
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net 3/4] net/smc: reset sndbuf_desc if freed
2020-09-03 19:53 [PATCH net 0/4] net/smc: fixes 2020-09-03 Karsten Graul
2020-09-03 19:53 ` [PATCH net 1/4] net/smc: fix toleration of fake add_link messages Karsten Graul
2020-09-03 19:53 ` [PATCH net 2/4] net/smc: set rx_off for SMCR explicitly Karsten Graul
@ 2020-09-03 19:53 ` Karsten Graul
2020-09-03 19:53 ` [PATCH net 4/4] net/smc: fix sock refcounting in case of termination Karsten Graul
2020-09-03 23:52 ` [PATCH net 0/4] net/smc: fixes 2020-09-03 David Miller
4 siblings, 0 replies; 6+ messages in thread
From: Karsten Graul @ 2020-09-03 19:53 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, heiko.carstens, raspl, ubraun
From: Ursula Braun <ubraun@linux.ibm.com>
When an SMC connection is created, and there is a problem to
create an RMB or DMB, the previously created send buffer is
thrown away as well including buffer descriptor freeing.
Make sure the connection no longer references the freed
buffer descriptor, otherwise bugs like this are possible:
[71556.835148] =============================================================================
[71556.835168] BUG kmalloc-128 (Tainted: G B OE ): Poison overwritten
[71556.835172] -----------------------------------------------------------------------------
[71556.835179] INFO: 0x00000000d20894be-0x00000000aaef63e9 @offset=2724. First byte 0x0 instead of 0x6b
[71556.835215] INFO: Allocated in __smc_buf_create+0x184/0x578 [smc] age=0 cpu=5 pid=46726
[71556.835234] ___slab_alloc+0x5a4/0x690
[71556.835239] __slab_alloc.constprop.0+0x70/0xb0
[71556.835243] kmem_cache_alloc_trace+0x38e/0x3f8
[71556.835250] __smc_buf_create+0x184/0x578 [smc]
[71556.835257] smc_buf_create+0x2e/0xe8 [smc]
[71556.835264] smc_listen_work+0x516/0x6a0 [smc]
[71556.835275] process_one_work+0x280/0x478
[71556.835280] worker_thread+0x66/0x368
[71556.835287] kthread+0x17a/0x1a0
[71556.835294] ret_from_fork+0x28/0x2c
[71556.835301] INFO: Freed in smc_buf_create+0xd8/0xe8 [smc] age=0 cpu=5 pid=46726
[71556.835307] __slab_free+0x246/0x560
[71556.835311] kfree+0x398/0x3f8
[71556.835318] smc_buf_create+0xd8/0xe8 [smc]
[71556.835324] smc_listen_work+0x516/0x6a0 [smc]
[71556.835328] process_one_work+0x280/0x478
[71556.835332] worker_thread+0x66/0x368
[71556.835337] kthread+0x17a/0x1a0
[71556.835344] ret_from_fork+0x28/0x2c
[71556.835348] INFO: Slab 0x00000000a0744551 objects=51 used=51 fp=0x0000000000000000 flags=0x1ffff00000010200
[71556.835352] INFO: Object 0x00000000563480a1 @offset=2688 fp=0x00000000289567b2
[71556.835359] Redzone 000000006783cde2: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb ................
[71556.835363] Redzone 00000000e35b876e: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb ................
[71556.835367] Redzone 0000000023074562: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb ................
[71556.835372] Redzone 00000000b9564b8c: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb ................
[71556.835376] Redzone 00000000810c6362: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb ................
[71556.835380] Redzone 0000000065ef52c3: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb ................
[71556.835384] Redzone 00000000c5dd6984: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb ................
[71556.835388] Redzone 000000004c480f8f: bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb ................
[71556.835392] Object 00000000563480a1: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
[71556.835397] Object 000000009c479d06: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
[71556.835401] Object 000000006e1dce92: 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b kkkk....kkkkkkkk
[71556.835405] Object 00000000227f7cf8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
[71556.835410] Object 000000009a701215: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
[71556.835414] Object 000000003731ce76: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
[71556.835418] Object 00000000f7085967: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
[71556.835422] Object 0000000007f99927: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5 kkkkkkkkkkkkkkk.
[71556.835427] Redzone 00000000579c4913: bb bb bb bb bb bb bb bb ........
[71556.835431] Padding 00000000305aef82: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ
[71556.835435] Padding 00000000b1cdd722: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ
[71556.835438] Padding 00000000c7568199: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ
[71556.835442] Padding 00000000fad4c4d4: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ
[71556.835451] CPU: 0 PID: 47939 Comm: kworker/0:15 Tainted: G B OE 5.9.0-rc1uschi+ #54
[71556.835456] Hardware name: IBM 3906 M03 703 (LPAR)
[71556.835464] Workqueue: events smc_listen_work [smc]
[71556.835470] Call Trace:
[71556.835478] [<00000000d5eaeb10>] show_stack+0x90/0xf8
[71556.835493] [<00000000d66fc0f8>] dump_stack+0xa8/0xe8
[71556.835499] [<00000000d61a511c>] check_bytes_and_report+0x104/0x130
[71556.835504] [<00000000d61a57b2>] check_object+0x26a/0x2e0
[71556.835509] [<00000000d61a59bc>] alloc_debug_processing+0x194/0x238
[71556.835514] [<00000000d61a8c14>] ___slab_alloc+0x5a4/0x690
[71556.835519] [<00000000d61a9170>] __slab_alloc.constprop.0+0x70/0xb0
[71556.835524] [<00000000d61aaf66>] kmem_cache_alloc_trace+0x38e/0x3f8
[71556.835530] [<000003ff80549bbc>] __smc_buf_create+0x184/0x578 [smc]
[71556.835538] [<000003ff8054a396>] smc_buf_create+0x2e/0xe8 [smc]
[71556.835545] [<000003ff80540c16>] smc_listen_work+0x516/0x6a0 [smc]
[71556.835549] [<00000000d5f0f448>] process_one_work+0x280/0x478
[71556.835554] [<00000000d5f0f6a6>] worker_thread+0x66/0x368
[71556.835559] [<00000000d5f18692>] kthread+0x17a/0x1a0
[71556.835563] [<00000000d6abf3b8>] ret_from_fork+0x28/0x2c
[71556.835569] INFO: lockdep is turned off.
[71556.835573] FIX kmalloc-128: Restoring 0x00000000d20894be-0x00000000aaef63e9=0x6b
[71556.835577] FIX kmalloc-128: Marking all objects used
Fixes: fd7f3a746582 ("net/smc: remove freed buffer from list")
Reviewed-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
---
net/smc/smc_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index a6ac0eb20c99..a406627b1d55 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -1779,6 +1779,7 @@ int smc_buf_create(struct smc_sock *smc, bool is_smcd)
list_del(&smc->conn.sndbuf_desc->list);
mutex_unlock(&smc->conn.lgr->sndbufs_lock);
smc_buf_free(smc->conn.lgr, false, smc->conn.sndbuf_desc);
+ smc->conn.sndbuf_desc = NULL;
}
return rc;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net 4/4] net/smc: fix sock refcounting in case of termination
2020-09-03 19:53 [PATCH net 0/4] net/smc: fixes 2020-09-03 Karsten Graul
` (2 preceding siblings ...)
2020-09-03 19:53 ` [PATCH net 3/4] net/smc: reset sndbuf_desc if freed Karsten Graul
@ 2020-09-03 19:53 ` Karsten Graul
2020-09-03 23:52 ` [PATCH net 0/4] net/smc: fixes 2020-09-03 David Miller
4 siblings, 0 replies; 6+ messages in thread
From: Karsten Graul @ 2020-09-03 19:53 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, heiko.carstens, raspl, ubraun
From: Ursula Braun <ubraun@linux.ibm.com>
When an ISM device is removed, all its linkgroups are terminated,
i.e. all the corresponding connections are killed.
Connection killing invokes smc_close_active_abort(), which decreases
the sock refcount for certain states to simulate passive closing.
And it cancels the close worker and has to give up the sock lock for
this timeframe. This opens the door for a passive close worker or a
socket close to run in between. In this case smc_close_active_abort() and
passive close worker resp. smc_release() might do a sock_put for passive
closing. This causes:
[ 1323.315943] refcount_t: underflow; use-after-free.
[ 1323.316055] WARNING: CPU: 3 PID: 54469 at lib/refcount.c:28 refcount_warn_saturate+0xe8/0x130
[ 1323.316069] Kernel panic - not syncing: panic_on_warn set ...
[ 1323.316084] CPU: 3 PID: 54469 Comm: uperf Not tainted 5.9.0-20200826.rc2.git0.46328853ed20.300.fc32.s390x+debug #1
[ 1323.316096] Hardware name: IBM 2964 NC9 702 (z/VM 6.4.0)
[ 1323.316108] Call Trace:
[ 1323.316125] [<00000000c0d4aae8>] show_stack+0x90/0xf8
[ 1323.316143] [<00000000c15989b0>] dump_stack+0xa8/0xe8
[ 1323.316158] [<00000000c0d8344e>] panic+0x11e/0x288
[ 1323.316173] [<00000000c0d83144>] __warn+0xac/0x158
[ 1323.316187] [<00000000c1597a7a>] report_bug+0xb2/0x130
[ 1323.316201] [<00000000c0d36424>] monitor_event_exception+0x44/0xc0
[ 1323.316219] [<00000000c195c716>] pgm_check_handler+0x1da/0x238
[ 1323.316234] [<00000000c151844c>] refcount_warn_saturate+0xec/0x130
[ 1323.316280] ([<00000000c1518448>] refcount_warn_saturate+0xe8/0x130)
[ 1323.316310] [<000003ff801f2e2a>] smc_release+0x192/0x1c8 [smc]
[ 1323.316323] [<00000000c169f1fa>] __sock_release+0x5a/0xe0
[ 1323.316334] [<00000000c169f2ac>] sock_close+0x2c/0x40
[ 1323.316350] [<00000000c1086de0>] __fput+0xb8/0x278
[ 1323.316362] [<00000000c0db1e0e>] task_work_run+0x76/0xb8
[ 1323.316393] [<00000000c0d8ab84>] do_exit+0x26c/0x520
[ 1323.316408] [<00000000c0d8af08>] do_group_exit+0x48/0xc0
[ 1323.316421] [<00000000c0d8afa8>] __s390x_sys_exit_group+0x28/0x38
[ 1323.316433] [<00000000c195c32c>] system_call+0xe0/0x2b4
[ 1323.316446] 1 lock held by uperf/54469:
[ 1323.316456] #0: 0000000044125e60 (&sb->s_type->i_mutex_key#9){+.+.}-{3:3}, at: __sock_release+0x44/0xe0
The patch rechecks sock state in smc_close_active_abort() after
smc_close_cancel_work() to avoid duplicate decrease of sock
refcount for the same purpose.
Fixes: 611b63a12732 ("net/smc: cancel tx worker in case of socket aborts")
Reviewed-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
---
net/smc/smc_close.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
index 290270c821ca..35e30d39510d 100644
--- a/net/smc/smc_close.c
+++ b/net/smc/smc_close.c
@@ -116,7 +116,6 @@ static void smc_close_cancel_work(struct smc_sock *smc)
cancel_work_sync(&smc->conn.close_work);
cancel_delayed_work_sync(&smc->conn.tx_work);
lock_sock(sk);
- sk->sk_state = SMC_CLOSED;
}
/* terminate smc socket abnormally - active abort
@@ -134,22 +133,22 @@ void smc_close_active_abort(struct smc_sock *smc)
}
switch (sk->sk_state) {
case SMC_ACTIVE:
- sk->sk_state = SMC_PEERABORTWAIT;
- smc_close_cancel_work(smc);
- sk->sk_state = SMC_CLOSED;
- sock_put(sk); /* passive closing */
- break;
case SMC_APPCLOSEWAIT1:
case SMC_APPCLOSEWAIT2:
+ sk->sk_state = SMC_PEERABORTWAIT;
smc_close_cancel_work(smc);
+ if (sk->sk_state != SMC_PEERABORTWAIT)
+ break;
sk->sk_state = SMC_CLOSED;
- sock_put(sk); /* postponed passive closing */
+ sock_put(sk); /* (postponed) passive closing */
break;
case SMC_PEERCLOSEWAIT1:
case SMC_PEERCLOSEWAIT2:
case SMC_PEERFINCLOSEWAIT:
sk->sk_state = SMC_PEERABORTWAIT;
smc_close_cancel_work(smc);
+ if (sk->sk_state != SMC_PEERABORTWAIT)
+ break;
sk->sk_state = SMC_CLOSED;
smc_conn_free(&smc->conn);
release_clcsock = true;
@@ -159,6 +158,8 @@ void smc_close_active_abort(struct smc_sock *smc)
case SMC_APPFINCLOSEWAIT:
sk->sk_state = SMC_PEERABORTWAIT;
smc_close_cancel_work(smc);
+ if (sk->sk_state != SMC_PEERABORTWAIT)
+ break;
sk->sk_state = SMC_CLOSED;
smc_conn_free(&smc->conn);
release_clcsock = true;
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net 0/4] net/smc: fixes 2020-09-03
2020-09-03 19:53 [PATCH net 0/4] net/smc: fixes 2020-09-03 Karsten Graul
` (3 preceding siblings ...)
2020-09-03 19:53 ` [PATCH net 4/4] net/smc: fix sock refcounting in case of termination Karsten Graul
@ 2020-09-03 23:52 ` David Miller
4 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-09-03 23:52 UTC (permalink / raw)
To: kgraul; +Cc: netdev, linux-s390, heiko.carstens, raspl, ubraun
From: Karsten Graul <kgraul@linux.ibm.com>
Date: Thu, 3 Sep 2020 21:53:14 +0200
> Please apply the following patch series for smc to netdev's net tree.
>
> Patch 1 fixes the toleration of older SMC implementations. Patch 2
> takes care of a problem that happens when SMCR is used after SMCD
> initialization failed. Patch 3 fixes a problem with freed send buffers,
> and patch 4 corrects refcounting when SMC terminates due to device
> removal.
Series applied, thank you.
^ permalink raw reply [flat|nested] 6+ messages in thread