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
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 <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
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
On Tue, Mar 17, 2020 at 1:45 AM Gabriel Krisman Bertazi
<[email protected]> 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