[PATCH 0/2] fixes to the hypervisor ubd thread

[PATCH 0/2] fixes to the hypervisor ubd thread

[Gabriel Krisman Bertazi]

Hi,

While debugging a somewhat related issue, I ran into two issues I
believe can cause the hypervisor to write garbage to the pipe.

This was find by visual inspection and is only slightly tested.  It
seems to partially some the problems my test case shows.

Please, let me know what you think

Gabriel Krisman Bertazi (2):
  um: ubd: Prevent buffer overrun on command completion
  um: ubd: Retry buffer read on any kind of error

 arch/um/drivers/ubd_kern.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

-- 
2.25.0

[PATCH 1/2] um: ubd: Prevent buffer overrun on command completion

[Gabriel Krisman Bertazi]

On the hypervisor side, when completing commands and the pipe is full,
we retry writing only the entries that failed, by offsetting
io_req_buffer, but we don't reduce the number of bytes written, which
can cause a buffer overrun of io_req_buffer, and write garbage to the
pipe.

Cc: Martyn Welch <martyn.welch@collabora.com>
Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
---
 arch/um/drivers/ubd_kern.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index 6627d7c30f37..0f5d0a699a49 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -1606,7 +1606,9 @@ int io_thread(void *arg)
 		written = 0;
 
 		do {
-			res = os_write_file(kernel_fd, ((char *) io_req_buffer) + written, n);
+			res = os_write_file(kernel_fd,
+					    ((char *) io_req_buffer) + written,
+					    n - written);
 			if (res >= 0) {
 				written += res;
 			}
-- 
2.25.0

[PATCH 2/2] um: ubd: Retry buffer read on any kind of error

[Gabriel Krisman Bertazi]

Should bulk_req_safe_read return an error, we want to retry the read,
otherwise, even though no IO will be done, os_write_file might still end
up writing garbage to the pipe.

Cc: Martyn Welch <martyn.welch@collabora.com>
Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
---
 arch/um/drivers/ubd_kern.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index 0f5d0a699a49..d259f0728003 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -1591,11 +1591,11 @@ int io_thread(void *arg)
 			&io_remainder_size,
 			UBD_REQ_BUFFER_SIZE
 		);
-		if (n < 0) {
-			if (n == -EAGAIN) {
+		if (n <= 0) {
+			if (n == -EAGAIN)
 				ubd_read_poll(-1);
-				continue;
-			}
+
+			continue;
 		}
 
 		for (count = 0; count < n/sizeof(struct io_thread_req *); count++) {
-- 
2.25.0

Re: [PATCH 0/2] fixes to the hypervisor ubd thread

[Richard Weinberger]

On Tue, Mar 17, 2020 at 1:45 AM Gabriel Krisman Bertazi
<krisman@collabora.com> wrote:
>
> Hi,
>
> While debugging a somewhat related issue, I ran into two issues I
> believe can cause the hypervisor to write garbage to the pipe.
>
> This was find by visual inspection and is only slightly tested.  It
> seems to partially some the problems my test case shows.
>
> Please, let me know what you think

Both patches make sense. Thanks for fixing, applied.

-- 
Thanks,
//richard