Date: Mon, 30 Oct 2006 17:49:36 GMT From: Paolo Pisati <piso@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 108766 for review Message-ID: <200610301749.k9UHna9Y091119@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=108766 Change 108766 by piso@piso_newluxor on 2006/10/30 17:49:03 Libalias locking: embed a sleep mtx per struct libalias instance, and grab/drop it at the entrance/exit of every public LibAlias* function. Not tested yet... Affected files ... .. //depot/projects/soc2005/libalias/sys/netinet/libalias/alias.c#19 edit .. //depot/projects/soc2005/libalias/sys/netinet/libalias/alias_db.c#13 edit .. //depot/projects/soc2005/libalias/sys/netinet/libalias/alias_local.h#8 edit .. //depot/projects/soc2005/libalias/sys/netinet/libalias/alias_proxy.c#9 edit .. //depot/projects/soc2005/libalias/sys/netinet/libalias/alias_util.c#4 edit Differences ... ==== //depot/projects/soc2005/libalias/sys/netinet/libalias/alias.c#19 (text+ko) ==== @@ -1138,6 +1138,7 @@ struct alias_link *lnk; struct ip *pip; + LIBALIAS_LOCK(la); pip = (struct ip *)ptr; lnk = AddFragmentPtrLink(la, pip->ip_src, pip->ip_id); iresult = PKT_ALIAS_ERROR; @@ -1145,6 +1146,7 @@ SetFragmentPtr(lnk, ptr); iresult = PKT_ALIAS_OK; } + LIBALIAS_UNLOCK(la); return (iresult); } @@ -1156,17 +1158,18 @@ char *fptr; struct ip *pip; + LIBALIAS_LOCK(la); pip = (struct ip *)ptr; lnk = FindFragmentPtr(la, pip->ip_src, pip->ip_id); if (lnk != NULL) { GetFragmentPtr(lnk, &fptr); SetFragmentPtr(lnk, NULL); SetExpire(lnk, 0); /* Deletes link */ + } else + fptr = NULL; - return (fptr); - } else { - return (NULL); - } + LIBALIAS_UNLOCK(la); + return (fptr); } @@ -1181,6 +1184,7 @@ struct ip *pip; struct ip *fpip; + LIBALIAS_LOCK(la); (void)la; pip = (struct ip *)ptr; fpip = (struct ip *)ptr_fragment; @@ -1188,6 +1192,7 @@ DifferentialChecksum(&fpip->ip_sum, &pip->ip_dst, &fpip->ip_dst, 2); fpip->ip_dst = pip->ip_dst; + LIBALIAS_UNLOCK(la); } @@ -1198,11 +1203,12 @@ struct ip *pip; int iresult; + LIBALIAS_LOCK(la); if (la->packetAliasMode & PKT_ALIAS_REVERSE) { la->packetAliasMode &= ~PKT_ALIAS_REVERSE; iresult = LibAliasOut(la, ptr, maxpacketsize); la->packetAliasMode |= PKT_ALIAS_REVERSE; - return (iresult); + goto getout; } HouseKeeping(la); ClearCheckNewLink(la); @@ -1211,8 +1217,10 @@ /* Defense against mangled packets */ if (ntohs(pip->ip_len) > maxpacketsize - || (pip->ip_hl << 2) > maxpacketsize) - return (PKT_ALIAS_IGNORED); + || (pip->ip_hl << 2) > maxpacketsize) { + iresult = PKT_ALIAS_IGNORED; + goto getout; + } iresult = PKT_ALIAS_IGNORED; if ((ntohs(pip->ip_off) & IP_OFFMASK) == 0) { @@ -1266,6 +1274,8 @@ iresult = FragmentIn(la, pip); } +getout: + LIBALIAS_UNLOCK(la); return (iresult); } @@ -1305,11 +1315,12 @@ struct in_addr addr_save; struct ip *pip; + LIBALIAS_LOCK(la); if (la->packetAliasMode & PKT_ALIAS_REVERSE) { la->packetAliasMode &= ~PKT_ALIAS_REVERSE; iresult = LibAliasIn(la, ptr, maxpacketsize); la->packetAliasMode |= PKT_ALIAS_REVERSE; - return (iresult); + goto getout; } HouseKeeping(la); ClearCheckNewLink(la); @@ -1317,8 +1328,10 @@ /* Defense against mangled packets */ if (ntohs(pip->ip_len) > maxpacketsize - || (pip->ip_hl << 2) > maxpacketsize) - return (PKT_ALIAS_IGNORED); + || (pip->ip_hl << 2) > maxpacketsize) { + iresult = PKT_ALIAS_IGNORED; + goto getout; + } addr_save = GetDefaultAliasAddress(la); if (la->packetAliasMode & PKT_ALIAS_UNREGISTERED_ONLY) { @@ -1380,6 +1393,8 @@ } SetDefaultAliasAddress(la, addr_save); +getout: + LIBALIAS_UNLOCK(la); return (iresult); } @@ -1395,12 +1410,13 @@ struct alias_link *lnk; int iresult = PKT_ALIAS_IGNORED; + LIBALIAS_LOCK(la); pip = (struct ip *)ptr; /* Defense against mangled packets */ if (ntohs(pip->ip_len) > maxpacketsize || (pip->ip_hl << 2) > maxpacketsize) - return (iresult); + goto getout; ud = (struct udphdr *)ip_next(pip); tc = (struct tcphdr *)ip_next(pip); @@ -1484,6 +1500,8 @@ iresult = PKT_ALIAS_OK; } } +getout: + LIBALIAS_UNLOCK(la); return (iresult); } ==== //depot/projects/soc2005/libalias/sys/netinet/libalias/alias_db.c#13 (text+ko) ==== @@ -2273,6 +2273,7 @@ int link_type; struct alias_link *lnk; + LIBALIAS_LOCK(la); switch (proto) { case IPPROTO_UDP: link_type = LINK_UDP; @@ -2285,7 +2286,8 @@ fprintf(stderr, "PacketAliasRedirectPort(): "); fprintf(stderr, "only TCP and UDP protocols allowed\n"); #endif - return (NULL); + lnk = NULL; + goto getout; } lnk = AddLink(la, src_addr, dst_addr, alias_addr, @@ -2302,6 +2304,8 @@ } #endif +getout: + LIBALIAS_UNLOCK(la); return (lnk); } @@ -2310,7 +2314,9 @@ LibAliasAddServer(struct libalias *la, struct alias_link *lnk, struct in_addr addr, u_short port) { struct server *server; + int res; + LIBALIAS_LOCK(la); (void)la; server = malloc(sizeof(struct server)); @@ -2332,9 +2338,12 @@ server->next = head; } lnk->server = server; - return (0); + res = 0; } else - return (-1); + res = -1; + + LIBALIAS_UNLOCK(la); + return (res); } /* Redirect packets of a given IP protocol from a specific @@ -2346,7 +2355,8 @@ u_char proto) { struct alias_link *lnk; - + + LIBALIAS_LOCK(la); lnk = AddLink(la, src_addr, dst_addr, alias_addr, NO_SRC_PORT, NO_DEST_PORT, 0, proto); @@ -2361,6 +2371,7 @@ } #endif + LIBALIAS_UNLOCK(la); return (lnk); } @@ -2371,6 +2382,7 @@ { struct alias_link *lnk; + LIBALIAS_LOCK(la); lnk = AddLink(la, src_addr, la->nullAddress, alias_addr, 0, 0, 0, LINK_ADDR); @@ -2385,6 +2397,7 @@ } #endif + LIBALIAS_UNLOCK(la); return (lnk); } @@ -2393,15 +2406,19 @@ int LibAliasRedirectDynamic(struct libalias *la, struct alias_link *lnk) { + int res; + LIBALIAS_LOCK(la); (void)la; if (lnk->flags & LINK_PARTIALLY_SPECIFIED) - return (-1); + res = -1; else { lnk->flags &= ~LINK_PERMANENT; - return (0); + res = 0; } + LIBALIAS_UNLOCK(la); + return (res); } @@ -2411,27 +2428,35 @@ /* This is a dangerous function to put in the API, because an invalid pointer can crash the program. */ + LIBALIAS_LOCK(la); la->deleteAllLinks = 1; DeleteLink(lnk); la->deleteAllLinks = 0; + LIBALIAS_UNLOCK(la); } void LibAliasSetAddress(struct libalias *la, struct in_addr addr) { + + LIBALIAS_LOCK(la); if (la->packetAliasMode & PKT_ALIAS_RESET_ON_ADDR_CHANGE && la->aliasAddress.s_addr != addr.s_addr) CleanupAliasData(la); la->aliasAddress = addr; + LIBALIAS_UNLOCK(la); } void LibAliasSetTarget(struct libalias *la, struct in_addr target_addr) { + + LIBALIAS_LOCK(la); la->targetAddress = target_addr; + LIBALIAS_UNLOCK(la); } static void @@ -2508,12 +2533,15 @@ #ifndef _KERNEL LibAliasRefreshModules(); #endif + LIBALIAS_LOCK_INIT(la); return (la); } void LibAliasUninit(struct libalias *la) { + + LIBALIAS_LOCK(la); la->deleteAllLinks = 1; CleanupAliasData(la); la->deleteAllLinks = 0; @@ -2522,6 +2550,8 @@ UninitPunchFW(la); #endif LIST_REMOVE(la, instancelist); + LIBALIAS_UNLOCK(la); + LIBALIAS_LOCK_DESTROY(la); free(la); } @@ -2534,11 +2564,15 @@ * do a probe for flag values) */ ) { + + LIBALIAS_LOCK(la); /* Enable logging? */ if (flags & mask & PKT_ALIAS_LOG) { /* Do the enable */ - if (InitPacketAliasLog(la) == ENOMEM) + if (InitPacketAliasLog(la) == ENOMEM) { + LIBALIAS_UNLOCK(la); return (-1); + } } else /* _Disable_ logging? */ if (~flags & mask & PKT_ALIAS_LOG) { @@ -2557,6 +2591,7 @@ /* Other flags can be set/cleared without special action */ la->packetAliasMode = (flags & mask) | (la->packetAliasMode & ~mask); + LIBALIAS_UNLOCK(la); return (la->packetAliasMode); } @@ -2564,7 +2599,12 @@ int LibAliasCheckNewLink(struct libalias *la) { - return (la->newDefaultLink); + int res; + + LIBALIAS_LOCK(la); + res = la->newDefaultLink; + LIBALIAS_UNLOCK(la); + return (res); } @@ -2819,14 +2859,20 @@ void LibAliasSetFWBase(struct libalias *la, unsigned int base, unsigned int num) { + + LIBALIAS_LOCK(la); #ifndef NO_FW_PUNCH la->fireWallBaseNum = base; la->fireWallNumNums = num; #endif + LIBALIAS_UNLOCK(la); } void LibAliasSetSkinnyPort(struct libalias *la, unsigned int port) { + + LIBALIAS_LOCK(la); la->skinnyPort = port; + LIBALIAS_UNLOCK(la); } ==== //depot/projects/soc2005/libalias/sys/netinet/libalias/alias_local.h#8 (text+ko) ==== @@ -51,7 +51,10 @@ #ifdef _KERNEL #include <sys/malloc.h> +#include <sys/param.h> #include <sys/lock.h> +#include <sys/mutex.h> + /* XXX: LibAliasSetTarget() uses this constant. */ #define INADDR_NONE 0xffffffff #endif @@ -146,11 +149,29 @@ struct in_addr true_addr; /* in network byte order. */ u_short true_port; /* in host byte order. */ - +#ifdef _KERNEL + /* + * avoid races in libalias: every pubblic function has to use it. + */ + struct mtx mutex; +#endif }; /* Macros */ +#ifdef _KERNEL +#define LIBALIAS_LOCK_INIT(l) \ + mtx_init(&l->mutex, "per-instance libalias mutex", NULL, MTX_DEF|MTX_RECURSE) +#define LIBALIAS_LOCK(l) mtx_lock(&l->mutex) +#define LIBALIAS_UNLOCK(l) mtx_unlock(&l->mutex) +#define LIBALIAS_LOCK_DESTROY(l) mtx_destroy(&l->mutex); +#else +#define LIBALIAS_LOCK_INIT(l) +#define LIBALIAS_LOCK(l) +#define LIBALIAS_UNLOCK(l) +#define LIBALIAS_LOCK_DESTROY(l) +#endif + /* * The following macro is used to update an * internet checksum. "delta" is a 32-bit ==== //depot/projects/soc2005/libalias/sys/netinet/libalias/alias_proxy.c#9 (text+ko) ==== @@ -648,7 +648,7 @@ * then 0 is used, and group 0 rules are always checked before any * others. */ - int i, n, len; + int i, n, len, ret; int cmd_len; int token_count; int state; @@ -668,11 +668,15 @@ struct in_addr dst_addr, dst_mask; struct proxy_entry *proxy_entry; + LIBALIAS_LOCK(la); + ret = 0; /* Copy command line into a buffer */ cmd += strspn(cmd, " \t"); cmd_len = strlen(cmd); - if (cmd_len > (int)(sizeof(buffer) - 1)) - return (-1); + if (cmd_len > (int)(sizeof(buffer) - 1)) { + ret = -1; + goto getout; + } strcpy(buffer, cmd); /* Convert to lower case */ @@ -730,8 +734,10 @@ state = STATE_READ_SRC; else if (strcmp(token, "dst") == 0) state = STATE_READ_DST; - else - return (-1); + else { + ret = -1; + goto getout; + } break; case STATE_READ_TYPE: @@ -741,8 +747,10 @@ proxy_type = PROXY_TYPE_ENCODE_TCPSTREAM; else if (strcmp(token, "no_encode") == 0) proxy_type = PROXY_TYPE_ENCODE_NONE; - else - return (-1); + else { + ret = -1; + goto getout; + } state = STATE_READ_KEYWORD; break; @@ -763,18 +771,24 @@ if (*p != ':') { err = IpAddr(token, &server_addr); - if (err) - return (-1); + if (err) { + ret = -1; + goto getout; + } } else { *p = ' '; n = sscanf(token, "%s %s", s, str_server_port); - if (n != 2) - return (-1); + if (n != 2) { + ret = -1; + goto getout; + } err = IpAddr(s, &server_addr); - if (err) - return (-1); + if (err) { + ret = -1; + goto getout; + } } } state = STATE_READ_KEYWORD; @@ -782,8 +796,10 @@ case STATE_READ_RULE: n = sscanf(token, "%d", &rule_index); - if (n != 1 || rule_index < 0) - return (-1); + if (n != 1 || rule_index < 0) { + ret = -1; + goto getout; + } state = STATE_READ_KEYWORD; break; @@ -792,16 +808,21 @@ int err; int rule_to_delete; - if (token_count != 2) - return (-1); + if (token_count != 2) { + ret = -1; + goto getout; + } n = sscanf(token, "%d", &rule_to_delete); - if (n != 1) - return (-1); + if (n != 1) { + ret = -1; + goto getout; + } err = RuleNumberDelete(la, rule_to_delete); if (err) - return (-1); - return (0); + ret = -1; + ret = 0; + goto getout; } case STATE_READ_PROTO: @@ -809,8 +830,10 @@ proto = IPPROTO_TCP; else if (strcmp(token, "udp") == 0) proto = IPPROTO_UDP; - else - return (-1); + else { + ret = -1; + goto getout; + } state = STATE_READ_KEYWORD; break; @@ -829,24 +852,32 @@ if (*p != '/') { IpMask(32, &mask); err = IpAddr(token, &addr); - if (err) - return (-1); + if (err) { + ret = -1; + goto getout; + } } else { int nbits; char s[sizeof(buffer)]; *p = ' '; n = sscanf(token, "%s %d", s, &nbits); - if (n != 2) - return (-1); + if (n != 2) { + ret = -1; + goto getout; + } err = IpAddr(s, &addr); - if (err) - return (-1); + if (err) { + ret = -1; + goto getout; + } err = IpMask(nbits, &mask); - if (err) - return (-1); + if (err) { + ret = -1; + goto getout; + } } if (state == STATE_READ_SRC) { @@ -861,7 +892,8 @@ break; default: - return (-1); + ret = -1; + goto getout; break; } @@ -887,8 +919,10 @@ int err; err = IpPort(str_port, proto, &proxy_port); - if (err) - return (-1); + if (err) { + ret = -1; + goto getout; + } } else { proxy_port = 0; } @@ -897,20 +931,26 @@ int err; err = IpPort(str_server_port, proto, &server_port); - if (err) - return (-1); + if (err) { + ret = -1; + goto getout; + } } else { server_port = 0; } /* Check that at least the server address has been defined */ - if (server_addr.s_addr == 0) - return (-1); + if (server_addr.s_addr == 0) { + ret = -1; + goto getout; + } /* Add to linked list */ proxy_entry = malloc(sizeof(struct proxy_entry)); - if (proxy_entry == NULL) - return (-1); + if (proxy_entry == NULL) { + ret = -1; + goto getout; + } proxy_entry->proxy_type = proxy_type; proxy_entry->rule_index = rule_index; @@ -925,5 +965,7 @@ RuleAdd(la, proxy_entry); - return (0); +getout: + LIBALIAS_UNLOCK(la); + return (ret); } ==== //depot/projects/soc2005/libalias/sys/netinet/libalias/alias_util.c#4 (text+ko) ==== @@ -76,6 +76,7 @@ { int sum, oddbyte; + LIBALIAS_LOCK(la); sum = 0; while (nbytes > 1) { sum += *ptr++; @@ -89,6 +90,7 @@ } sum = (sum >> 16) + (sum & 0xffff); sum += (sum >> 16); + LIBALIAS_UNLOCK(la); return (~sum); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200610301749.k9UHna9Y091119>