Fix data corruption bug

Fix data corruption bug

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
Subject: [PATCH] chnet: Fix data corruption when a read operation is pending

When a read operation is pending, the @read_ret_ variable has to be
set to -EINPROGRESS. The current CHNetDelegate::__Read() method
doesn't do that. This situation makes the caller abort the request
because it doesn't see -EINPROGRESS from the return value. This bug
is reproducible on a slow network connection where __Read()
operation often results in a pending state.

This bug is introduced in commit 77bbcc903899 ("chnet: Completely
refactor again").

Fixes: 77bbcc903899 ("chnet: Completely refactor again")
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 chnet/chnet.cc | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/chnet/chnet.cc b/chnet/chnet.cc
index cd27810..fdbbdc6 100644
--- a/chnet/chnet.cc
+++ b/chnet/chnet.cc
@@ -713,9 +713,14 @@ int CHNetDelegate::__Read(int size)
 
 	read_buf_ = base::MakeRefCounted<net::IOBufferWithSize>(size);
 	ret = url_req_->Read(read_buf_.get(), size);
-	read_ret_.store(ret, std::memory_order_release);
 
-	if (ret != net::ERR_IO_PENDING && callback_.on_read_completed_)
+	if (ret == net::ERR_IO_PENDING) {
+		read_ret_.store(-EINPROGRESS, std::memory_order_release);
+		return ret;
+	}
+
+	read_ret_.store(ret, std::memory_order_release);
+	if (callback_.on_read_completed_)
 		std::move(callback_.on_read_completed_)(url_req_.get(), ret);
 
 	return ret;
-- 
Ammar Faizi

Re: Fix data corruption bug

[Alviro Iskandar Setiawan]

On Fri, Nov 18, 2022 at 12:27 AM Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> Subject: [PATCH] chnet: Fix data corruption when a read operation is pending
>
> When a read operation is pending, the @read_ret_ variable has to be
> set to -EINPROGRESS. The current CHNetDelegate::__Read() method
> doesn't do that. This situation makes the caller abort the request
> because it doesn't see -EINPROGRESS from the return value. This bug
> is reproducible on a slow network connection where __Read()
> operation often results in a pending state.
>
> This bug is introduced in commit 77bbcc903899 ("chnet: Completely
> refactor again").
>
> Fixes: 77bbcc903899 ("chnet: Completely refactor again")
> Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Reviewed-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>

tq

-- Viro

Re: Fix data corruption bug

[Kyoko Senpai]

On Fri, Nov 18, 2022 at 9:31 AM Alviro Iskandar Setiawan wrote:
> On Fri, Nov 18, 2022 at 12:27 AM Ammar Faizi wrote:
> > From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> > Subject: [PATCH] chnet: Fix data corruption when a read operation is pending
> >
> > When a read operation is pending, the @read_ret_ variable has to be
> > set to -EINPROGRESS. The current CHNetDelegate::__Read() method
> > doesn't do that. This situation makes the caller abort the request
> > because it doesn't see -EINPROGRESS from the return value. This bug
> > is reproducible on a slow network connection where __Read()
> > operation often results in a pending state.
> >
> > This bug is introduced in commit 77bbcc903899 ("chnet: Completely
> > refactor again").
> >
> > Fixes: 77bbcc903899 ("chnet: Completely refactor again")
> > Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
>
> Reviewed-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>

Reviewed-by: Gilang Fachrezy <gilang4321@gmail.com>