Re: integer truncation in soreceive()

2016-01-01 Thread Stefan Kempf
Thanks. A similar diff was discussed privately with a few
developers during the last few days and is about to be
committed soon.

Martin Natano wrote:
> Another integer overflow: A recv() call with a size of 2^32 bytes causes
> soreceive() to spin in an endless loop, resulting in a system freeze.
> The diff below prevents this behaviour by establishing an upper bound
> for uio_resid before assigning the value to an integer variable with
> smaller width. Also the 'offset' and 'resid' variables are converted to
> use the correct integer types.
> 
> cheers,
> natano
> 
> Index: kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.144
> diff -u -p -u -r1.144 uipc_socket.c
> --- kern/uipc_socket.c5 Dec 2015 10:11:53 -   1.144
> +++ kern/uipc_socket.c31 Dec 2015 21:26:01 -
> @@ -613,13 +613,14 @@ soreceive(struct socket *so, struct mbuf
>  {
>   struct mbuf *m, **mp;
>   struct mbuf *cm;
> - int flags, len, error, s, offset;
> + int flags, len, error, s;
> + u_long offset;
>   struct protosw *pr = so->so_proto;
>   struct mbuf *nextrecord;
>   int moff, type = 0;
>   size_t orig_resid = uio->uio_resid;
>   int uio_error = 0;
> - int resid;
> + size_t resid;
>  
>   mp = mp0;
>   if (paddr)
> @@ -639,8 +640,8 @@ soreceive(struct socket *so, struct mbuf
>   if (error)
>   goto bad;
>   do {
> - error = uiomovei(mtod(m, caddr_t),
> - (int) min(uio->uio_resid, m->m_len), uio);
> + error = uiomove(mtod(m, caddr_t),
> + ulmin(uio->uio_resid, m->m_len), uio);
>   m = m_free(m);
>   } while (uio->uio_resid && error == 0 && m);
>  bad:
> @@ -833,11 +834,9 @@ dontblock:
>   panic("receive 3");
>  #endif
>   so->so_state &= ~SS_RCVATMARK;
> - len = uio->uio_resid;
> + len = ulmin(uio->uio_resid, m->m_len - moff);
>   if (so->so_oobmark && len > so->so_oobmark - offset)
>   len = so->so_oobmark - offset;
> - if (len > m->m_len - moff)
> - len = m->m_len - moff;
>   /*
>* If mp is set, just pass back the mbufs.
>* Otherwise copy them out via the uio, then free.
> 



integer truncation in soreceive()

2015-12-31 Thread Martin Natano
Another integer overflow: A recv() call with a size of 2^32 bytes causes
soreceive() to spin in an endless loop, resulting in a system freeze.
The diff below prevents this behaviour by establishing an upper bound
for uio_resid before assigning the value to an integer variable with
smaller width. Also the 'offset' and 'resid' variables are converted to
use the correct integer types.

cheers,
natano

Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.144
diff -u -p -u -r1.144 uipc_socket.c
--- kern/uipc_socket.c  5 Dec 2015 10:11:53 -   1.144
+++ kern/uipc_socket.c  31 Dec 2015 21:26:01 -
@@ -613,13 +613,14 @@ soreceive(struct socket *so, struct mbuf
 {
struct mbuf *m, **mp;
struct mbuf *cm;
-   int flags, len, error, s, offset;
+   int flags, len, error, s;
+   u_long offset;
struct protosw *pr = so->so_proto;
struct mbuf *nextrecord;
int moff, type = 0;
size_t orig_resid = uio->uio_resid;
int uio_error = 0;
-   int resid;
+   size_t resid;
 
mp = mp0;
if (paddr)
@@ -639,8 +640,8 @@ soreceive(struct socket *so, struct mbuf
if (error)
goto bad;
do {
-   error = uiomovei(mtod(m, caddr_t),
-   (int) min(uio->uio_resid, m->m_len), uio);
+   error = uiomove(mtod(m, caddr_t),
+   ulmin(uio->uio_resid, m->m_len), uio);
m = m_free(m);
} while (uio->uio_resid && error == 0 && m);
 bad:
@@ -833,11 +834,9 @@ dontblock:
panic("receive 3");
 #endif
so->so_state &= ~SS_RCVATMARK;
-   len = uio->uio_resid;
+   len = ulmin(uio->uio_resid, m->m_len - moff);
if (so->so_oobmark && len > so->so_oobmark - offset)
len = so->so_oobmark - offset;
-   if (len > m->m_len - moff)
-   len = m->m_len - moff;
/*
 * If mp is set, just pass back the mbufs.
 * Otherwise copy them out via the uio, then free.