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
[email protected]
On Thu, Aug 20, 2020 at 7:18 AM Commit Bot <[email protected]> wrote:
> From: Waldemar Kozaczuk <[email protected]>
> Committer: Waldemar Kozaczuk <[email protected]>
> 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 <[email protected]>
>
> ---
> 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
> --- 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
>
>
> --
> 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 [email protected].
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/osv-dev/000000000000b1e63b05ad476a5e%40google.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 [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/osv-dev/CANEVyjuGYogd1ZqtYZzyWXFYuXA6Jcv69PO%2BtT7skL4wv7g8XQ%40mail.gmail.com.
