5.3-RELEASE TODO
Brooks Davis
brooks at one-eyed-alien.net
Fri Sep 17 22:16:31 PDT 2004
On Fri, Sep 17, 2004 at 09:30:19PM +0200, Dag-Erling Smørgrav wrote:
> Brooks Davis <brooks at 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.
>
> 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:13 2004
+++ sys/net/if.c Fri Sep 17 22:13:06 2004
@@ -36,9 +36,11 @@
#include "opt_mac.h"
#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 = (struct ifconf *)data;
struct ifnet *ifp;
struct ifaddr *ifa;
- struct ifreq ifr, *ifrp;
- int space = ifc->ifc_len, error = 0;
+ struct ifreq ifr;
+ struct sbuf *sb;
+
+ /* Limit buffer size to MAXPHYS to avoid DoS from userspace. */
+ sb = sbuf_new(NULL, NULL,
+ (ifc->ifc_len >= MAXPHYS ? MAXPHYS : (ifc->ifc_len + 1)),
+ SBUF_FIXEDLEN);
- ifrp = ifc->ifc_req;
IFNET_RLOCK(); /* could sleep XXX */
TAILQ_FOREACH(ifp, &ifnet, if_link) {
int addrs;
- if (space < sizeof(ifr))
- break;
if (strlcpy(ifr.ifr_name, ifp->if_xname, sizeof(ifr.ifr_name))
- >= sizeof(ifr.ifr_name)) {
- error = ENAMETOOLONG;
- break;
- }
+ >= sizeof(ifr.ifr_name))
+ return (ENAMETOOLONG);
addrs = 0;
TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
struct sockaddr *sa = ifa->ifa_addr;
- 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 = *sa;
osa->sa_family = sa->sa_family;
- error = copyout((caddr_t)&ifr, (caddr_t)ifrp,
- sizeof (ifr));
- ifrp++;
+ sbuf_bcat(sb, &ifr, sizeof(ifr));
} else
#endif
if (sa->sa_len <= sizeof(*sa)) {
ifr.ifr_addr = *sa;
- error = 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 -= sa->sa_len - sizeof(*sa);
- error = copyout((caddr_t)&ifr, (caddr_t)ifrp,
- sizeof (ifr.ifr_name));
- if (error == 0)
- error = copyout((caddr_t)sa,
- (caddr_t)&ifrp->ifr_addr, sa->sa_len);
- ifrp = (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 -= sizeof (ifr);
+ ifc->ifc_len = sbuf_len(sb);
}
- if (error)
- break;
- if (!addrs) {
+ if (addrs == 0) {
bzero((caddr_t)&ifr.ifr_addr, sizeof(ifr.ifr_addr));
- error = copyout((caddr_t)&ifr, (caddr_t)ifrp,
- sizeof (ifr));
- if (error)
+ sbuf_bcat(sb, &ifr, sizeof(ifr));
+
+ if (sbuf_overflowed(sb))
break;
- space -= sizeof (ifr);
- ifrp++;
+ ifc->ifc_len = sbuf_len(sb);
}
}
IFNET_RUNLOCK();
- ifc->ifc_len -= space;
- return (error);
+
+ return (copyout(sbuf_data(sb), ifc->ifc_req, ifc->ifc_len));
}
/*
--
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-current/attachments/20040917/7f9e7ca6/attachment.bin
More information about the freebsd-current
mailing list