For what it's worth, your diff reads fine to me. It took me a while to
realize, that tlen in pppxwrite() and tun_dev_write() can't overflow
because of the range check beforehand, but on close look that code also
turns out to be correct.

cheers,
natano


On Fri, Jan 22, 2016 at 07:29:54PM +0100, Stefan Kempf wrote:
> The following diff prevents integer truncation of uio_resid
> by using ulmin() instead of min() and calls uiomove() instead
> of the legacy uiomove().
> 
> That's straightforward because the m_len mbuf field is unsigned.
> mlen can be turned to a size_t because it's set to MLEN or MCLBYTES,
> which is > 0.
> 
> I plan to commit this in a few days unless there are objections.
> 
> Index: if_pppx.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_pppx.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 if_pppx.c
> --- if_pppx.c 14 Jan 2016 09:20:31 -0000      1.49
> +++ if_pppx.c 22 Jan 2016 18:23:04 -0000
> @@ -273,7 +273,8 @@ pppxread(dev_t dev, struct uio *uio, int
>       struct pppx_dev *pxd = pppx_dev2pxd(dev);
>       struct mbuf *m, *m0;
>       int error = 0;
> -     int len, s;
> +     int s;
> +     size_t len;
>  
>       if (!pxd)
>               return (ENXIO);
> @@ -292,9 +293,9 @@ pppxread(dev_t dev, struct uio *uio, int
>       }
>  
>       while (m0 != NULL && uio->uio_resid > 0 && error == 0) {
> -             len = min(uio->uio_resid, m0->m_len);
> +             len = ulmin(uio->uio_resid, m0->m_len);
>               if (len != 0)
> -                     error = uiomovei(mtod(m0, caddr_t), len, uio);
> +                     error = uiomove(mtod(m0, caddr_t), len, uio);
>               m = m_free(m0);
>               m0 = m;
>       }
> @@ -313,8 +314,9 @@ pppxwrite(dev_t dev, struct uio *uio, in
>       uint32_t proto;
>       struct mbuf *top, **mp, *m;
>       struct niqueue *ifq;
> -     int tlen, mlen;
> +     int tlen;
>       int error = 0;
> +     size_t mlen;
>  #if NBPFILTER > 0
>       int s;
>  #endif
> @@ -342,8 +344,8 @@ pppxwrite(dev_t dev, struct uio *uio, in
>       mp = ⊤
>  
>       while (error == 0 && uio->uio_resid > 0) {
> -             m->m_len = min(mlen, uio->uio_resid);
> -             error = uiomovei(mtod (m, caddr_t), m->m_len, uio);
> +             m->m_len = ulmin(mlen, uio->uio_resid);
> +             error = uiomove(mtod (m, caddr_t), m->m_len, uio);
>               *mp = m;
>               mp = &m->m_next;
>               if (error == 0 && uio->uio_resid > 0) {
> Index: if_tun.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_tun.c,v
> retrieving revision 1.165
> diff -u -p -r1.165 if_tun.c
> --- if_tun.c  7 Jan 2016 05:31:17 -0000       1.165
> +++ if_tun.c  22 Jan 2016 18:23:05 -0000
> @@ -764,7 +764,8 @@ tun_dev_read(struct tun_softc *tp, struc
>       struct ifnet            *ifp = &tp->tun_if;
>       struct mbuf             *m, *m0;
>       unsigned int             ifidx;
> -     int                      error = 0, len, s;
> +     int                      error = 0, s;
> +     size_t                   len;
>  
>       if ((tp->tun_flags & TUN_READY) != TUN_READY)
>               return (EHOSTDOWN);
> @@ -825,9 +826,9 @@ tun_dev_read(struct tun_softc *tp, struc
>       }
>  
>       while (m0 != NULL && uio->uio_resid > 0 && error == 0) {
> -             len = min(uio->uio_resid, m0->m_len);
> +             len = ulmin(uio->uio_resid, m0->m_len);
>               if (len != 0)
> -                     error = uiomovei(mtod(m0, caddr_t), len, uio);
> +                     error = uiomove(mtod(m0, caddr_t), len, uio);
>               m = m_free(m0);
>               m0 = m;
>       }
> @@ -872,7 +873,8 @@ tun_dev_write(struct tun_softc *tp, stru
>       struct niqueue          *ifq;
>       u_int32_t               *th;
>       struct mbuf             *top, **mp, *m;
> -     int                      error=0, tlen, mlen;
> +     int                     error = 0, tlen;
> +     size_t                  mlen;
>  #if NBPFILTER > 0
>       int                      s;
>  #endif
> @@ -911,8 +913,8 @@ tun_dev_write(struct tun_softc *tp, stru
>               m->m_data += ETHER_ALIGN;
>       }
>       while (error == 0 && uio->uio_resid > 0) {
> -             m->m_len = min(mlen, uio->uio_resid);
> -             error = uiomovei(mtod (m, caddr_t), m->m_len, uio);
> +             m->m_len = ulmin(mlen, uio->uio_resid);
> +             error = uiomove(mtod (m, caddr_t), m->m_len, uio);
>               *mp = m;
>               mp = &m->m_next;
>               if (error == 0 && uio->uio_resid > 0) {
> 

Reply via email to