Date: Fri, 17 Sep 2004 22:17:59 -0700 From: Brooks Davis <brooks@one-eyed-alien.net> To: Dag-Erling =?iso-8859-1?Q?Sm=F8rgrav?= <des@des.no> Cc: current@FreeBSD.org Subject: Re: 5.3-RELEASE TODO Message-ID: <20040918051758.GA18656@odin.ac.hmc.edu> In-Reply-To: <xzpllf8n16c.fsf@dwp.des.no> References: <200409170741.i8H7fGV3011078@pooker.samsco.org> <20040917162535.GA5750@odin.ac.hmc.edu> <xzpu0twn5gz.fsf@dwp.des.no> <20040917184817.GA22747@odin.ac.hmc.edu> <xzpllf8n16c.fsf@dwp.des.no>
next in thread | previous in thread | raw e-mail | index | archive | help
--r5Pyd7+fXNt84Ff3
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Fri, Sep 17, 2004 at 09:30:19PM +0200, Dag-Erling Sm=F8rgrav wrote:
> Brooks Davis <brooks@one-eyed-alien.net> writes:
> > Thus, so you have to know how much space you will
> > need before doing any kind of allocation, hence the double loop and the
> > potential race.
>=20
> Using sbufs removes the need for loop and greatly simplifies how you
> deal with overflows.
Indeed it does. I'm not fully happy with the hardcoding of a maximum
size, but I doubt anyone will hit it in practice. Here's a new and
improved patch that makes a single pass and uses sbufs.
-- Brooks
--- /home/brooks/working/freebsd/p4/freebsd/sys/net/if.c Fri Sep 17 22:11:1=
3 2004
+++ sys/net/if.c Fri Sep 17 22:13:06 2004
@@ -36,9 +36,11 @@
#include "opt_mac.h"
=20
#include <sys/param.h>
+#include <sys/types.h>
#include <sys/conf.h>
#include <sys/mac.h>
#include <sys/malloc.h>
+#include <sys/sbuf.h>
#include <sys/bus.h>
#include <sys/mbuf.h>
#include <sys/systm.h>
@@ -1483,28 +1485,26 @@
struct ifconf *ifc =3D (struct ifconf *)data;
struct ifnet *ifp;
struct ifaddr *ifa;
- struct ifreq ifr, *ifrp;
- int space =3D ifc->ifc_len, error =3D 0;
+ struct ifreq ifr;
+ struct sbuf *sb;
+
+ /* Limit buffer size to MAXPHYS to avoid DoS from userspace. */
+ sb =3D sbuf_new(NULL, NULL,
+ (ifc->ifc_len >=3D MAXPHYS ? MAXPHYS : (ifc->ifc_len + 1)),
+ SBUF_FIXEDLEN);
=20
- ifrp =3D ifc->ifc_req;
IFNET_RLOCK(); /* could sleep XXX */
TAILQ_FOREACH(ifp, &ifnet, if_link) {
int addrs;
=20
- if (space < sizeof(ifr))
- break;
if (strlcpy(ifr.ifr_name, ifp->if_xname, sizeof(ifr.ifr_name))
- >=3D sizeof(ifr.ifr_name)) {
- error =3D ENAMETOOLONG;
- break;
- }
+ >=3D sizeof(ifr.ifr_name))
+ return (ENAMETOOLONG);
=20
addrs =3D 0;
TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
struct sockaddr *sa =3D ifa->ifa_addr;
=20
- if (space < sizeof(ifr))
- break;
if (jailed(curthread->td_ucred) &&
prison_if(curthread->td_ucred, sa))
continue;
@@ -1515,48 +1515,34 @@
(struct osockaddr *)&ifr.ifr_addr;
ifr.ifr_addr =3D *sa;
osa->sa_family =3D sa->sa_family;
- error =3D copyout((caddr_t)&ifr, (caddr_t)ifrp,
- sizeof (ifr));
- ifrp++;
+ sbuf_bcat(sb, &ifr, sizeof(ifr));
} else
#endif
if (sa->sa_len <=3D sizeof(*sa)) {
ifr.ifr_addr =3D *sa;
- error =3D copyout((caddr_t)&ifr, (caddr_t)ifrp,
- sizeof (ifr));
- ifrp++;
+ sbuf_bcat(sb, &ifr, sizeof(ifr));
} else {
- if (space < sizeof (ifr) + sa->sa_len -
- sizeof(*sa))
- break;
- space -=3D sa->sa_len - sizeof(*sa);
- error =3D copyout((caddr_t)&ifr, (caddr_t)ifrp,
- sizeof (ifr.ifr_name));
- if (error =3D=3D 0)
- error =3D copyout((caddr_t)sa,
- (caddr_t)&ifrp->ifr_addr, sa->sa_len);
- ifrp =3D (struct ifreq *)
- (sa->sa_len + (caddr_t)&ifrp->ifr_addr);
+ sbuf_bcat(sb, &ifr,
+ offsetof(struct ifreq, ifr_addr));
+ sbuf_bcat(sb, sa, sa->sa_len);
}
- if (error)
+
+ if (sbuf_overflowed(sb))
break;
- space -=3D sizeof (ifr);
+ ifc->ifc_len =3D sbuf_len(sb);
}
- if (error)
- break;
- if (!addrs) {
+ if (addrs =3D=3D 0) {
bzero((caddr_t)&ifr.ifr_addr, sizeof(ifr.ifr_addr));
- error =3D copyout((caddr_t)&ifr, (caddr_t)ifrp,
- sizeof (ifr));
- if (error)
+ sbuf_bcat(sb, &ifr, sizeof(ifr));
+
+ if (sbuf_overflowed(sb))
break;
- space -=3D sizeof (ifr);
- ifrp++;
+ ifc->ifc_len =3D sbuf_len(sb);
}
}
IFNET_RUNLOCK();
- ifc->ifc_len -=3D space;
- return (error);
+
+ return (copyout(sbuf_data(sb), ifc->ifc_req, ifc->ifc_len));
}
=20
/*
--=20
Any statement of the form "X is the one, true Y" is FALSE.
PGP fingerprint 655D 519C 26A7 82E7 2529 9BF0 5D8E 8BE9 F238 1AD4
--r5Pyd7+fXNt84Ff3
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)
iD8DBQFBS8UGXY6L6fI4GtQRAhLEAJ4okkmQGwfqDDDgaedxAo8Tg6hBbQCeOm5H
EXbFNcIMAuylGBJn8xz9fKk=
=IQEp
-----END PGP SIGNATURE-----
--r5Pyd7+fXNt84Ff3--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040918051758.GA18656>