* [PATCH net] net/smc: fix application data exception
@ 2022-11-26 8:22 D.Wythe
2023-02-14 12:14 ` D. Wythe
2023-02-15 9:27 ` Wenjia Zhang
0 siblings, 2 replies; 5+ messages in thread
From: D.Wythe @ 2022-11-26 8:22 UTC (permalink / raw)
To: kgraul, wenjia, jaka; +Cc: kuba, davem, netdev, linux-s390, linux-rdma
From: "D. Wythe" <alibuda@linux.alibaba.com>
There is a certain probability that following
exceptions will occur in the wrk benchmark test:
Running 10s test @ http://11.213.45.6:80
8 threads and 64 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 3.72ms 13.94ms 245.33ms 94.17%
Req/Sec 1.96k 713.67 5.41k 75.16%
155262 requests in 10.10s, 23.10MB read
Non-2xx or 3xx responses: 3
We will find that the error is HTTP 400 error, which is a serious
exception in our test, which means the application data was
corrupted.
Consider the following scenarios:
CPU0 CPU1
buf_desc->used = 0;
cmpxchg(buf_desc->used, 0, 1)
deal_with(buf_desc)
memset(buf_desc->cpu_addr,0);
This will cause the data received by a victim connection to be cleared,
thus triggering an HTTP 400 error in the server.
This patch exchange the order between clear used and memset, add
barrier to ensure memory consistency.
Fixes: 1c5526968e27 ("net/smc: Clear memory when release and reuse buffer")
Signed-off-by: D. Wythe <alibuda@linux.alibaba.com>
---
net/smc/smc_core.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index c305d8d..c19d4b7 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -1120,8 +1120,9 @@ static void smcr_buf_unuse(struct smc_buf_desc *buf_desc, bool is_rmb,
smc_buf_free(lgr, is_rmb, buf_desc);
} else {
- buf_desc->used = 0;
- memset(buf_desc->cpu_addr, 0, buf_desc->len);
+ /* memzero_explicit provides potential memory barrier semantics */
+ memzero_explicit(buf_desc->cpu_addr, buf_desc->len);
+ WRITE_ONCE(buf_desc->used, 0);
}
}
@@ -1132,19 +1133,17 @@ static void smc_buf_unuse(struct smc_connection *conn,
if (!lgr->is_smcd && conn->sndbuf_desc->is_vm) {
smcr_buf_unuse(conn->sndbuf_desc, false, lgr);
} else {
- conn->sndbuf_desc->used = 0;
- memset(conn->sndbuf_desc->cpu_addr, 0,
- conn->sndbuf_desc->len);
+ memzero_explicit(conn->sndbuf_desc->cpu_addr, conn->sndbuf_desc->len);
+ WRITE_ONCE(conn->sndbuf_desc->used, 0);
}
}
if (conn->rmb_desc) {
if (!lgr->is_smcd) {
smcr_buf_unuse(conn->rmb_desc, true, lgr);
} else {
- conn->rmb_desc->used = 0;
- memset(conn->rmb_desc->cpu_addr, 0,
- conn->rmb_desc->len +
- sizeof(struct smcd_cdc_msg));
+ memzero_explicit(conn->rmb_desc->cpu_addr,
+ conn->rmb_desc->len + sizeof(struct smcd_cdc_msg));
+ WRITE_ONCE(conn->rmb_desc->used, 0);
}
}
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net] net/smc: fix application data exception
2022-11-26 8:22 [PATCH net] net/smc: fix application data exception D.Wythe
@ 2023-02-14 12:14 ` D. Wythe
2023-02-15 9:27 ` Wenjia Zhang
1 sibling, 0 replies; 5+ messages in thread
From: D. Wythe @ 2023-02-14 12:14 UTC (permalink / raw)
To: kgraul, wenjia, jaka; +Cc: kuba, davem, netdev, linux-s390, linux-rdma
Hi, wenjia
This patch of bugfix seems to have been hanging for a long time.
If you have any concerns, please let us know.
Best wishes.
D. Wythe
On 11/26/22 4:22 PM, D.Wythe wrote:
> From: "D. Wythe" <alibuda@linux.alibaba.com>
>
> There is a certain probability that following
> exceptions will occur in the wrk benchmark test:
>
> Running 10s test @ http://11.213.45.6:80
> 8 threads and 64 connections
> Thread Stats Avg Stdev Max +/- Stdev
> Latency 3.72ms 13.94ms 245.33ms 94.17%
> Req/Sec 1.96k 713.67 5.41k 75.16%
> 155262 requests in 10.10s, 23.10MB read
> Non-2xx or 3xx responses: 3
>
> We will find that the error is HTTP 400 error, which is a serious
> exception in our test, which means the application data was
> corrupted.
>
> Consider the following scenarios:
>
> CPU0 CPU1
>
> buf_desc->used = 0;
> cmpxchg(buf_desc->used, 0, 1)
> deal_with(buf_desc)
>
> memset(buf_desc->cpu_addr,0);
>
> This will cause the data received by a victim connection to be cleared,
> thus triggering an HTTP 400 error in the server.
>
> This patch exchange the order between clear used and memset, add
> barrier to ensure memory consistency.
>
> Fixes: 1c5526968e27 ("net/smc: Clear memory when release and reuse buffer")
> Signed-off-by: D. Wythe <alibuda@linux.alibaba.com>
> ---
> net/smc/smc_core.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index c305d8d..c19d4b7 100644
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c
> @@ -1120,8 +1120,9 @@ static void smcr_buf_unuse(struct smc_buf_desc *buf_desc, bool is_rmb,
>
> smc_buf_free(lgr, is_rmb, buf_desc);
> } else {
> - buf_desc->used = 0;
> - memset(buf_desc->cpu_addr, 0, buf_desc->len);
> + /* memzero_explicit provides potential memory barrier semantics */
> + memzero_explicit(buf_desc->cpu_addr, buf_desc->len);
> + WRITE_ONCE(buf_desc->used, 0);
> }
> }
>
> @@ -1132,19 +1133,17 @@ static void smc_buf_unuse(struct smc_connection *conn,
> if (!lgr->is_smcd && conn->sndbuf_desc->is_vm) {
> smcr_buf_unuse(conn->sndbuf_desc, false, lgr);
> } else {
> - conn->sndbuf_desc->used = 0;
> - memset(conn->sndbuf_desc->cpu_addr, 0,
> - conn->sndbuf_desc->len);
> + memzero_explicit(conn->sndbuf_desc->cpu_addr, conn->sndbuf_desc->len);
> + WRITE_ONCE(conn->sndbuf_desc->used, 0);
> }
> }
> if (conn->rmb_desc) {
> if (!lgr->is_smcd) {
> smcr_buf_unuse(conn->rmb_desc, true, lgr);
> } else {
> - conn->rmb_desc->used = 0;
> - memset(conn->rmb_desc->cpu_addr, 0,
> - conn->rmb_desc->len +
> - sizeof(struct smcd_cdc_msg));
> + memzero_explicit(conn->rmb_desc->cpu_addr,
> + conn->rmb_desc->len + sizeof(struct smcd_cdc_msg));
> + WRITE_ONCE(conn->rmb_desc->used, 0);
> }
> }
> }
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] net/smc: fix application data exception
2022-11-26 8:22 [PATCH net] net/smc: fix application data exception D.Wythe
2023-02-14 12:14 ` D. Wythe
@ 2023-02-15 9:27 ` Wenjia Zhang
2023-02-15 17:31 ` Jakub Kicinski
1 sibling, 1 reply; 5+ messages in thread
From: Wenjia Zhang @ 2023-02-15 9:27 UTC (permalink / raw)
To: D.Wythe, kgraul, jaka; +Cc: kuba, davem, netdev, linux-s390, linux-rdma
On 26.11.22 09:22, D.Wythe wrote:
> From: "D. Wythe" <alibuda@linux.alibaba.com>
>
> There is a certain probability that following
> exceptions will occur in the wrk benchmark test:
>
> Running 10s test @ http://11.213.45.6:80
> 8 threads and 64 connections
> Thread Stats Avg Stdev Max +/- Stdev
> Latency 3.72ms 13.94ms 245.33ms 94.17%
> Req/Sec 1.96k 713.67 5.41k 75.16%
> 155262 requests in 10.10s, 23.10MB read
> Non-2xx or 3xx responses: 3
>
> We will find that the error is HTTP 400 error, which is a serious
> exception in our test, which means the application data was
> corrupted.
>
> Consider the following scenarios:
>
> CPU0 CPU1
>
> buf_desc->used = 0;
> cmpxchg(buf_desc->used, 0, 1)
> deal_with(buf_desc)
>
> memset(buf_desc->cpu_addr,0);
>
> This will cause the data received by a victim connection to be cleared,
> thus triggering an HTTP 400 error in the server.
>
> This patch exchange the order between clear used and memset, add
> barrier to ensure memory consistency.
>
> Fixes: 1c5526968e27 ("net/smc: Clear memory when release and reuse buffer")
> Signed-off-by: D. Wythe <alibuda@linux.alibaba.com>
> ---
> net/smc/smc_core.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index c305d8d..c19d4b7 100644
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c
> @@ -1120,8 +1120,9 @@ static void smcr_buf_unuse(struct smc_buf_desc *buf_desc, bool is_rmb,
>
> smc_buf_free(lgr, is_rmb, buf_desc);
> } else {
> - buf_desc->used = 0;
> - memset(buf_desc->cpu_addr, 0, buf_desc->len);
> + /* memzero_explicit provides potential memory barrier semantics */
> + memzero_explicit(buf_desc->cpu_addr, buf_desc->len);
> + WRITE_ONCE(buf_desc->used, 0);
> }
> }
>
> @@ -1132,19 +1133,17 @@ static void smc_buf_unuse(struct smc_connection *conn,
> if (!lgr->is_smcd && conn->sndbuf_desc->is_vm) {
> smcr_buf_unuse(conn->sndbuf_desc, false, lgr);
> } else {
> - conn->sndbuf_desc->used = 0;
> - memset(conn->sndbuf_desc->cpu_addr, 0,
> - conn->sndbuf_desc->len);
> + memzero_explicit(conn->sndbuf_desc->cpu_addr, conn->sndbuf_desc->len);
> + WRITE_ONCE(conn->sndbuf_desc->used, 0);
> }
> }
> if (conn->rmb_desc) {
> if (!lgr->is_smcd) {
> smcr_buf_unuse(conn->rmb_desc, true, lgr);
> } else {
> - conn->rmb_desc->used = 0;
> - memset(conn->rmb_desc->cpu_addr, 0,
> - conn->rmb_desc->len +
> - sizeof(struct smcd_cdc_msg));
> + memzero_explicit(conn->rmb_desc->cpu_addr,
> + conn->rmb_desc->len + sizeof(struct smcd_cdc_msg));
> + WRITE_ONCE(conn->rmb_desc->used, 0);
> }
> }
> }
Hi David,
Thank you for remembering me again about this patch. I did forget to
answer you, sorry!
My consideration was if memzero_explicit() is necessary in this case.
But sure, it makes sense, especiall when the dereferencing is in
somewhere else.
Thank you for the fix!
Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] net/smc: fix application data exception
2023-02-15 9:27 ` Wenjia Zhang
@ 2023-02-15 17:31 ` Jakub Kicinski
2023-02-16 4:14 ` D. Wythe
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2023-02-15 17:31 UTC (permalink / raw)
To: Wenjia Zhang; +Cc: D.Wythe, kgraul, jaka, davem, netdev, linux-s390, linux-rdma
On Wed, 15 Feb 2023 10:27:55 +0100 Wenjia Zhang wrote:
> Hi David,
>
> Thank you for remembering me again about this patch. I did forget to
> answer you, sorry!
>
> My consideration was if memzero_explicit() is necessary in this case.
> But sure, it makes sense, especiall when the dereferencing is in
> somewhere else.
>
> Thank you for the fix!
>
> Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com>
Thanks! David, please repost if you'd like the patch to be applied to
the networking tree. The original posting is too old to use.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] net/smc: fix application data exception
2023-02-15 17:31 ` Jakub Kicinski
@ 2023-02-16 4:14 ` D. Wythe
0 siblings, 0 replies; 5+ messages in thread
From: D. Wythe @ 2023-02-16 4:14 UTC (permalink / raw)
To: Jakub Kicinski, Wenjia Zhang
Cc: kgraul, jaka, davem, netdev, linux-s390, linux-rdma
On 2/16/23 1:31 AM, Jakub Kicinski wrote:
> On Wed, 15 Feb 2023 10:27:55 +0100 Wenjia Zhang wrote:
>> Hi David,
>>
>> Thank you for remembering me again about this patch. I did forget to
>> answer you, sorry!
>>
>> My consideration was if memzero_explicit() is necessary in this case.
>> But sure, it makes sense, especiall when the dereferencing is in
>> somewhere else.
>>
>> Thank you for the fix!
>>
>> Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com>
>
> Thanks! David, please repost if you'd like the patch to be applied to
> the networking tree. The original posting is too old to use.
Thank you for your reminder. I will repost it after rebasing.
D. Wythe
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-02-16 4:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-26 8:22 [PATCH net] net/smc: fix application data exception D.Wythe
2023-02-14 12:14 ` D. Wythe
2023-02-15 9:27 ` Wenjia Zhang
2023-02-15 17:31 ` Jakub Kicinski
2023-02-16 4:14 ` D. Wythe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).