[PATCH] fs:change uiomove() to use size_t in the signature instead of int

24 views
Skip to first unread message

Waldemar Kozaczuk

unread,
Aug 17, 2020, 12:00:23 PM8/17/20
to osv...@googlegroups.com, Waldemar Kozaczuk
As Wonsup Yoon reported in the issue #1095 following C++ trying
to read large number of bytes from /dev/random fails:

```
#include<fstream>
#include<vector>
#include<iostream>

#define READ_BYTES (1LL << 31)

int main(){

std::ifstream file("/dev/urandom");

std::vector<char> data(READ_BYTES);
data[0] = 1;
file.read(data.data(), READ_BYTES);

std::cout << +data[0] << std::endl;


return 0;
}
```

To make long story short (see the issue for more details)
it turns out that the culprit is wron signature of uiomove()
function that uses int instead of size_t for n parameter.
This patch fixes it.

Fixes #1095

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---
fs/vfs/subr_uio.cc | 4 ++--
include/osv/uio.h | 8 ++------
2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/fs/vfs/subr_uio.cc b/fs/vfs/subr_uio.cc
index bf138b8e..fdc89bd4 100644
--- a/fs/vfs/subr_uio.cc
+++ b/fs/vfs/subr_uio.cc
@@ -41,13 +41,13 @@
#include <osv/uio.h>

int
-uiomove(void *cp, int n, struct uio *uio)
+uiomove(void *cp, size_t n, struct uio *uio)
{
assert(uio->uio_rw == UIO_READ || uio->uio_rw == UIO_WRITE);

while (n > 0 && uio->uio_resid) {
struct iovec *iov = uio->uio_iov;
- int cnt = iov->iov_len;
+ size_t cnt = iov->iov_len;
if (cnt == 0) {
uio->uio_iov++;
uio->uio_iovcnt--;
diff --git a/include/osv/uio.h b/include/osv/uio.h
index 8e2951be..2c117e7c 100644
--- a/include/osv/uio.h
+++ b/include/osv/uio.h
@@ -42,11 +42,7 @@ __BEGIN_DECLS

enum uio_rw { UIO_READ, UIO_WRITE };

-/*
- * Safe default to prevent possible overflows in user code, otherwise could
- * be SSIZE_T_MAX.
- */
-#define IOSIZE_MAX INT_MAX
+#define IOSIZE_MAX SSIZE_MAX

#undef UIO_MAXIOV
#define UIO_MAXIOV 1024
@@ -61,7 +57,7 @@ struct uio {
enum uio_rw uio_rw; /* operation */
};

-int uiomove(void *cp, int n, struct uio *uio);
+int uiomove(void *cp, size_t n, struct uio *uio);

__END_DECLS

--
2.26.2

Wonsup Yoon

unread,
Aug 18, 2020, 1:30:37 AM8/18/20
to OSv Development
Yes, it is fixed.

2020년 8월 18일 화요일 오전 1시 0분 23초 UTC+9에 jwkoz...@gmail.com님이 작성:

Commit Bot

unread,
Aug 20, 2020, 12:18:49 AM8/20/20
to osv...@googlegroups.com, Waldemar Kozaczuk
From: Waldemar Kozaczuk <jwkoz...@gmail.com>
Committer: Waldemar Kozaczuk <jwkoz...@gmail.com>
Branch: master

fs:change uiomove() to use size_t in the signature instead of int

As Wonsup Yoon reported in the issue #1095 following C++ trying
to read large number of bytes from /dev/random fails:

```
#include<fstream>
#include<vector>
#include<iostream>

#define READ_BYTES (1LL << 31)

int main(){

std::ifstream file("/dev/urandom");

std::vector<char> data(READ_BYTES);
data[0] = 1;
file.read(data.data(), READ_BYTES);

std::cout << +data[0] << std::endl;

return 0;
}
```

To make long story short (see the issue for more details)
it turns out that the culprit is wron signature of uiomove()
function that uses int instead of size_t for n parameter.
This patch fixes it.

Fixes #1095

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>

---
diff --git a/fs/vfs/subr_uio.cc b/fs/vfs/subr_uio.cc
--- a/fs/vfs/subr_uio.cc
+++ b/fs/vfs/subr_uio.cc
@@ -41,13 +41,13 @@
#include <osv/uio.h>

int
-uiomove(void *cp, int n, struct uio *uio)
+uiomove(void *cp, size_t n, struct uio *uio)
{
assert(uio->uio_rw == UIO_READ || uio->uio_rw == UIO_WRITE);

while (n > 0 && uio->uio_resid) {
struct iovec *iov = uio->uio_iov;
- int cnt = iov->iov_len;
+ size_t cnt = iov->iov_len;
if (cnt == 0) {
uio->uio_iov++;
uio->uio_iovcnt--;
diff --git a/include/osv/uio.h b/include/osv/uio.h

Nadav Har'El

unread,
Aug 20, 2020, 4:05:41 AM8/20/20
to Waldemar Kozaczuk, Osv Dev
Thanks. Looks good.

I agree that using SSIZE_MAX as the maximum instead of SIZE_MAX (which you could have used) is indeed a good idea - it will prevent somebody from accidentally storing a large number in a small integer and then having it wrongly sign-extended, e.g.,
          
     int x = 4294967295 // looks like a large number, but gets treated as "-1".
     uiomove(..., x)        // x sign extended to 64-bit "-1", i.e., 18446744073709551616, will refuse to work because > SSIZE_MAX, so good.

--
Nadav Har'El
n...@scylladb.com


--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/000000000000b1e63b05ad476a5e%40google.com.
Reply all
Reply to author
Forward
0 new messages