PERFORCE change 177677 for review
Marko Zec
zec at icir.org
Mon May 3 22:18:38 UTC 2010
On Monday 03 May 2010 23:03:49 Ana Kukec wrote:
> http://p4web.freebsd.org/@@177677?ac=10
>
> Change 177677 by anchie at anchie_malimis on 2010/05/03 21:03:44
>
> Getting rid of the global variable V_send_so from files other then
> send.[ch].
Just wondering - isn't this change actually increasing the possibility for a
race between packet datapath and send.ko kldloading / kldunloading? I.e. had
we kept the V_send_so variable global, we could (at least with more
confidence) avoid calling into send hooks when they are in an intermitent
state purely by checking whether V_send_so is NULL. Now we can't tell
whether another thread is just in the process of kldunloading the send module
while we are calling into one of the send hooks in our thread.
Also, if I'm not mistaking, even if send_sendso_input() returns an error, it
will have the mbuf consumed / freed, so any further attempts to do anything
with the mbuf will crash the system. Do we really want to do m_freem(m) at
the bottom of send_sendso_input()?
Marko
> Affected files ...
>
> .. //depot/projects/soc2009/anchie_send/src/sys/netinet6/icmp6.c#39 edit
> .. //depot/projects/soc2009/anchie_send/src/sys/netinet6/nd6.c#29 edit
> .. //depot/projects/soc2009/anchie_send/src/sys/netinet6/nd6_nbr.c#16 edit
> .. //depot/projects/soc2009/anchie_send/src/sys/netinet6/raw_ip6.c#10 edit
> .. //depot/projects/soc2009/anchie_send/src/sys/netinet6/send.c#43 edit
> .. //depot/projects/soc2009/anchie_send/src/sys/netinet6/send.h#21 edit
>
> Differences ...
>
> ==== //depot/projects/soc2009/anchie_send/src/sys/netinet6/icmp6.c#39
> (text+ko) ====
>
> @@ -424,7 +424,7 @@
> int icmp6len = m->m_pkthdr.len - *offp;
> int code, sum, noff;
> char ip6bufs[INET6_ADDRSTRLEN], ip6bufd[INET6_ADDRSTRLEN];
> - int ip6len;
> + int ip6len, error = -1;
>
> ifp = m->m_pkthdr.rcvif;
>
> @@ -780,24 +780,37 @@
> /* give up local */
>
> /* Send incoming SeND-protected/ND packet to user space. */
> - if (send_sendso_input_hook != NULL && V_send_so != NULL) {
> - IP6_EXTHDR_CHECK(m, off, icmp6len, IPPROTO_DONE);
> - printf("send_sendso_input_hook\n");
> - send_sendso_input_hook(V_send_so, m, SND_IN, ip6len);
> - } else {
> + if (send_sendso_input_hook != NULL) {
> + IP6_EXTHDR_CHECK(m, off,
> + icmp6len, IPPROTO_DONE);
> + error = send_sendso_input_hook(m,
> + SND_IN, ip6len);
> + /* -1 == no app on SEND socket */
> + if (!error)
> + return (IPPROTO_DONE);
> + }
> + if ((send_sendso_input_hook != NULL
> + && error == -1) ||
> + send_sendso_input_hook == NULL) {
> /* give up local */
> nd6_rs_input(m, off, icmp6len);
> }
> m = NULL;
> goto freeit;
> }
> - if (send_sendso_input_hook != NULL && V_send_so != NULL) {
> - IP6_EXTHDR_CHECK(m, off, icmp6len, IPPROTO_DONE);
> - printf("send_sendso_input_hook\n");
> - send_sendso_input_hook(V_send_so, n, SND_IN,
> ip6len); - return (IPPROTO_DONE);
> - } else
> + if (send_sendso_input_hook != NULL) {
> + IP6_EXTHDR_CHECK(m, off,
> + icmp6len, IPPROTO_DONE);
> + error = send_sendso_input_hook(n,
> + SND_IN, ip6len);
> + /* -1 == no app on SEND socket */
> + if (!error)
> + return (IPPROTO_DONE);
> + }
> + if ((send_sendso_input_hook != NULL && error == -1)
> + || (send_sendso_input_hook == NULL)) {
> nd6_rs_input(n, off, icmp6len);
> + }
> /* m stays. */
> break;
>
> @@ -810,20 +823,30 @@
> if ((n = m_copym(m, 0, M_COPYALL, M_DONTWAIT)) == NULL) {
>
> /* Send incoming SeND-protected/ND packet to user space. */
> - if (send_sendso_input_hook != NULL && V_send_so != NULL) {
> -
> - send_sendso_input_hook(V_send_so, m, SND_IN, ip6len);
> - return (IPPROTO_DONE);
> - } else
> + if (send_sendso_input_hook != NULL) {
> + error = send_sendso_input_hook(m,
> + SND_IN, ip6len);
> + if (!error)
> + return (IPPROTO_DONE);
> + }
> + if ((send_sendso_input_hook != NULL
> + && error == -1) ||
> + send_sendso_input_hook == NULL) {
> nd6_ra_input(m, off, icmp6len);
> + }
> m = NULL;
> goto freeit;
> }
> - if (send_sendso_input_hook != NULL && V_send_so != NULL) {
> - send_sendso_input_hook(V_send_so, n, SND_IN, ip6len);
> - return (IPPROTO_DONE);
> - } else
> + if (send_sendso_input_hook != NULL) {
> + error = send_sendso_input_hook(n,
> + SND_IN, ip6len);
> + if (!error)
> + return (IPPROTO_DONE);
> + }
> + if ((send_sendso_input_hook != NULL && error == -1)
> + || (send_sendso_input_hook == NULL)) {
> nd6_ra_input(n, off, icmp6len);
> + }
> /* m stays. */
> break;
>
> @@ -834,23 +857,27 @@
> if (icmp6len < sizeof(struct nd_neighbor_solicit))
> goto badlen;
> if ((n = m_copym(m, 0, M_COPYALL, M_DONTWAIT)) == NULL) {
> - if (send_sendso_input_hook != NULL && V_send_so != NULL) {
> - /* Send incoming SeND/ND packet to user space. */
> - printf("%s: send_sendso_input_hook m=%p\n", __func__, m);
> - send_sendso_input_hook(V_send_so, m, SND_IN, ip6len);
> - } else {
> + if (send_sendso_input_hook != NULL) {
> + error = send_sendso_input_hook(m,
> + SND_IN, ip6len);
> + }
> + if ((send_sendso_input_hook != NULL
> + && error == -1) ||
> + send_sendso_input_hook == NULL) {
> /* give up local */
> nd6_ns_input(m, off, icmp6len);
> }
> m = NULL;
> goto freeit;
> }
> - if (send_sendso_input_hook != NULL && V_send_so != NULL) {
> - /* Send incoming SeND/ND packet to user space. */
> - printf("%s: send_sendso_input_hook n=%p\n", __func__, n);
> - send_sendso_input_hook(V_send_so, n, SND_IN, ip6len);
> - } else
> + if (send_sendso_input_hook != NULL) {
> + error = send_sendso_input_hook(n,
> + SND_IN, ip6len);
> + }
> + if ((send_sendso_input_hook != NULL && error == -1)
> + || (send_sendso_input_hook == NULL)) {
> nd6_ns_input(n, off, icmp6len);
> + }
> /* m stays. */
> break;
>
> @@ -863,20 +890,29 @@
> if ((n = m_copym(m, 0, M_COPYALL, M_DONTWAIT)) == NULL) {
>
> /* Send incoming SeND-protected/ND packet to user space. */
> - if (send_sendso_input_hook != NULL && V_send_so != NULL) {
> - send_sendso_input_hook(V_send_so, m, SND_IN, ip6len);
> - return (IPPROTO_DONE);
> - } else {
> + if (send_sendso_input_hook != NULL) {
> + error = send_sendso_input_hook(m,
> + SND_IN, ip6len);
> + if (!error)
> + return (IPPROTO_DONE);
> + }
> + if ((send_sendso_input_hook != NULL
> + && error == -1) ||
> + send_sendso_input_hook == NULL) {
> /* give up local */
> nd6_na_input(m, off, icmp6len);
> }
> m = NULL;
> goto freeit;
> }
> - if (send_sendso_input_hook != NULL)
> - send_sendso_input_hook(V_send_so, n, SND_IN, ip6len);
> - else
> + if (send_sendso_input_hook != NULL) {
> + error = send_sendso_input_hook(n,
> + SND_IN, ip6len);
> + }
> + if ((send_sendso_input_hook != NULL && error == -1)
> + || (send_sendso_input_hook == NULL)) {
> nd6_na_input(n, off, icmp6len);
> + }
> /* m stays. */
> break;
>
> @@ -887,23 +923,35 @@
> if (icmp6len < sizeof(struct nd_redirect))
> goto badlen;
> if ((n = m_copym(m, 0, M_COPYALL, M_DONTWAIT)) == NULL) {
> -
> - /* Send incoming SeND-protected/ND packet to user space. */
> - if (send_sendso_input_hook != NULL && V_send_so != NULL) {
> - send_sendso_input_hook(V_send_so, m, SND_IN, ip6len);
> - return (IPPROTO_DONE);
> - } else {
> + if (send_sendso_input_hook != NULL) {
> + error = send_sendso_input_hook(m,
> + SND_IN, ip6len);
> + if (!error)
> + return (IPPROTO_DONE);
> + else
> + goto freeit;
> + }
> + if ((send_sendso_input_hook != NULL
> + && error == -1) ||
> + send_sendso_input_hook == NULL) {
> /* give up local */
> icmp6_redirect_input(m, off);
> }
> m = NULL;
> goto freeit;
> }
> - if (send_sendso_input_hook != NULL && V_send_so != NULL) {
> - send_sendso_input_hook(V_send_so, n, SND_IN, ip6len);
> - return (IPPROTO_DONE);
> - } else
> + if (send_sendso_input_hook != NULL) {
> + error = send_sendso_input_hook(n,
> + SND_IN, ip6len);
> + if (!error)
> + return (IPPROTO_DONE);
> + else
> + goto freeit;
> + }
> + if ((send_sendso_input_hook != NULL && error == -1)
> + || (send_sendso_input_hook == NULL)) {
> icmp6_redirect_input(n, off);
> + }
> /* m stays. */
> break;
>
> @@ -2805,7 +2853,7 @@
> nd_rd->nd_rd_cksum = in6_cksum(m, IPPROTO_ICMPV6,
> sizeof(*ip6), ntohs(ip6->ip6_plen));
>
> - if (send_sendso_input_hook != NULL && V_send_so != NULL) {
> + if (send_sendso_input_hook != NULL) {
> mtag = m_tag_get(PACKET_TAG_ND_OUTGOING, sizeof(unsigned short),
> M_NOWAIT);
> if (mtag == NULL)
>
> ==== //depot/projects/soc2009/anchie_send/src/sys/netinet6/nd6.c#29
> (text+ko) ====
>
> @@ -113,7 +113,7 @@
>
> static struct sockaddr_in6 all1_sa;
>
> -int (*send_sendso_input_hook)(struct socket *, struct mbuf *, int, int);
> +int (*send_sendso_input_hook)(struct mbuf *, int, int);
>
> static int nd6_is_new_addr_neighbor __P((struct sockaddr_in6 *,
> struct ifnet *));
> @@ -1803,9 +1803,10 @@
> struct m_tag *mtag;
> struct llentry *ln = lle;
> struct ip6_hdr *ip6;
> - int error = 0;
> + int error = -1;
> int flags = 0;
> - int ip6len, skip = 0;
> + int ip6len;
> + int skip;
> unsigned short *nd_type;
>
> ip6 = mtod(m, struct ip6_hdr *);
> @@ -1985,15 +1986,19 @@
> mac_netinet6_nd6_send(ifp, m);
> #endif
>
> + skip = 0;
> /* send outgoing NS/NA/REDIRECT packet to sendd. */
> - if (send_sendso_input_hook != NULL && V_send_so != NULL) {
> + if (send_sendso_input_hook != NULL) {
> mtag = m_tag_find(m, PACKET_TAG_ND_OUTGOING, NULL);
> if (mtag != NULL) {
> skip = 1;
> nd_type = (unsigned short *)(mtag + 1);
> /* Use the SEND socket */
> - return (send_sendso_input_hook(V_send_so,
> - m, SND_OUT, ip6len);
> + error = send_sendso_input_hook(m, SND_OUT,
> + ip6len);
> + /* -1 == no app on SEND socket */
> + if (error == 0 && error != -1)
> + return (error);
> }
> }
>
>
> ==== //depot/projects/soc2009/anchie_send/src/sys/netinet6/nd6_nbr.c#16
> (text+ko) ====
>
> @@ -569,9 +569,9 @@
> nd_ns->nd_ns_cksum =
> in6_cksum(m, IPPROTO_ICMPV6, sizeof(*ip6), icmp6len);
>
> - if (send_sendso_input_hook != NULL && V_send_so != NULL) {
> - mtag = m_tag_get(PACKET_TAG_ND_OUTGOING, sizeof(unsigned short),
> - M_NOWAIT);
> + if (send_sendso_input_hook != NULL) {
> + mtag = m_tag_get(PACKET_TAG_ND_OUTGOING,
> + sizeof(unsigned short), M_NOWAIT);
> if (mtag == NULL)
> goto bad;
> *(unsigned short *)(mtag + 1) = nd_ns->nd_ns_type;
> @@ -896,7 +896,7 @@
> * the 2nd argument as the 1st one.
> */
>
> - if (send_sendso_input_hook != NULL && V_send_so != NULL) {
> + if (send_sendso_input_hook != NULL) {
> mtag = m_tag_get(PACKET_TAG_ND_OUTGOING,
> sizeof(unsigned short), M_NOWAIT);
> if (mtag == NULL)
> @@ -1091,8 +1091,9 @@
> nd_na->nd_na_cksum =
> in6_cksum(m, IPPROTO_ICMPV6, sizeof(struct ip6_hdr), icmp6len);
>
> - if (send_sendso_input_hook != NULL && V_send_so != NULL) {
> - mtag = m_tag_get(PACKET_TAG_ND_OUTGOING, sizeof(unsigned short),
> + if (send_sendso_input_hook != NULL) {
> + mtag = m_tag_get(PACKET_TAG_ND_OUTGOING,
> + sizeof(unsigned short),
> M_NOWAIT);
> if (mtag == NULL)
> goto bad;
>
> ==== //depot/projects/soc2009/anchie_send/src/sys/netinet6/raw_ip6.c#10
> (text+ko) ====
>
> @@ -535,8 +535,7 @@
> * Send RA/RS messages to user land for protection, before sending
> * them to rtadvd/rtsol.
> */
> - if (send_sendso_input_hook != NULL &&
> - V_send_so != NULL &&
> + if ((send_sendso_input_hook != NULL) &&
> so->so_proto->pr_protocol == IPPROTO_ICMPV6) {
> switch (type) {
> case ND_ROUTER_ADVERT:
>
> ==== //depot/projects/soc2009/anchie_send/src/sys/netinet6/send.c#43
> (text+ko) ====
>
> @@ -195,12 +195,15 @@
> * Send a message to the SEND daemon on the SEND socket.
> */
> static int
> -send_sendso_input(struct socket *s, struct mbuf *m, int direction, int
> msglen) +send_sendso_input(struct mbuf *m, int direction, int msglen)
> {
> u_int len;
> struct ip6_hdr *ip6;
> struct snd_hdr *snd_hdr = NULL;
>
> + if (V_send_so == NULL)
> + goto freeit;
> +
> /*
> * Make sure to clear any possible internally embedded scope before
> * passing the packet to userspace for SeND cryptographic signature
> @@ -225,15 +228,14 @@
> * protected (outgoing) or validated (incoming) according to rfc3971.
> */
>
> - if (s) {
> - SOCKBUF_LOCK(&s->so_rcv);
> - sbappendrecord_locked(&s->so_rcv, m);
> - sorwakeup_locked(s);
> - return (0);
> - }
> + SOCKBUF_LOCK(&s->so_rcv);
> + sbappendrecord_locked(&s->so_rcv, m);
> + sorwakeup_locked(s);
> + return (0);
>
> +freeit:
> m_freem(m);
> - return -1;
> + return (-1);
> }
>
> static void
>
> ==== //depot/projects/soc2009/anchie_send/src/sys/netinet6/send.h#21
> (text+ko) ====
>
> @@ -34,4 +34,4 @@
> int ifidx;
> };
>
> -extern int (*send_sendso_input_hook)(struct socket *, struct mbuf *,
> int, int); +extern int (*send_sendso_input_hook)(struct mbuf *, int,
> int);
More information about the p4-projects
mailing list