On 04/07/16(Mon) 15:52, Alexander Bluhm wrote:
> On Mon, Jul 04, 2016 at 01:03:22PM +0200, Martin Pieuchot wrote:
> > +   if (ip6_hbhchcheck(m, &off, &nxt, &ours)) {
> > +           if_put(ifp);
> > +           return; /* m have already been freed */
> >     }
> 
> As ip6_hbhchcheck() does ip6 = mtod(m, struct ip6_hdr *) after
> ip6_hopopts_input() you have to add this here, too.
> 
>       /* adjust pointer */
>       ip6 = mtod(m, struct ip6_hdr *);

Updated thanks!

> > +int
> > +ip6_hbhchcheck(struct mbuf *m, int *offp, int *nxtp, int *oursp)
> > +{
> > +   struct ip6_hdr *ip6;
> > +   u_int32_t plen, rtalert = ~0;
> > +   int ours, off, nxt;
> 
> ours may be used uninitialized.
> 
> > +   *offp = off;
> > +   *nxtp = nxt;
> > +   *oursp = ours;
> 
> I would prefer to use the passed values as *off, *nxt, *ours directly
> than to use another set of local variables.  This also fixes
> initialization problem.

Fine, new diff doing that.

Index: netinet6/ip6_input.c
===================================================================
RCS file: /cvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.161
diff -u -p -r1.161 ip6_input.c
--- netinet6/ip6_input.c        5 Jul 2016 10:17:14 -0000       1.161
+++ netinet6/ip6_input.c        5 Jul 2016 10:21:10 -0000
@@ -122,6 +122,7 @@ struct ip6stat ip6stat;
 void ip6_init2(void *);
 int ip6_check_rh0hdr(struct mbuf *, int *);
 
+int ip6_hbhchcheck(struct mbuf *, int *, int *, int *);
 int ip6_hopopts_input(u_int32_t *, u_int32_t *, struct mbuf **, int *);
 struct mbuf *ip6_pullexthdr(struct mbuf *, size_t, int);
 
@@ -192,7 +193,6 @@ ip6_input(struct mbuf *m)
        struct ip6_hdr *ip6;
        int off, nest;
        u_int16_t src_scope, dst_scope;
-       u_int32_t plen, rtalert = ~0;
        int nxt, ours = 0;
 #if NPF > 0
        struct in6_addr odst;
@@ -495,78 +495,15 @@ ip6_input(struct mbuf *m)
        }
 
   hbhcheck:
-       /*
-        * Process Hop-by-Hop options header if it's contained.
-        * m may be modified in ip6_hopopts_input().
-        * If a JumboPayload option is included, plen will also be modified.
-        */
-       plen = (u_int32_t)ntohs(ip6->ip6_plen);
-       off = sizeof(struct ip6_hdr);
-       if (ip6->ip6_nxt == IPPROTO_HOPOPTS) {
-               struct ip6_hbh *hbh;
-
-               if (ip6_hopopts_input(&plen, &rtalert, &m, &off)) {
-                       if_put(ifp);
-                       return; /* m have already been freed */
-               }
-
-               /* adjust pointer */
-               ip6 = mtod(m, struct ip6_hdr *);
-
-               /*
-                * if the payload length field is 0 and the next header field
-                * indicates Hop-by-Hop Options header, then a Jumbo Payload
-                * option MUST be included.
-                */
-               if (ip6->ip6_plen == 0 && plen == 0) {
-                       /*
-                        * Note that if a valid jumbo payload option is
-                        * contained, ip6_hopopts_input() must set a valid
-                        * (non-zero) payload length to the variable plen.
-                        */
-                       ip6stat.ip6s_badoptions++;
-                       icmp6_error(m, ICMP6_PARAM_PROB,
-                                   ICMP6_PARAMPROB_HEADER,
-                                   (caddr_t)&ip6->ip6_plen - (caddr_t)ip6);
-                       if_put(ifp);
-                       return;
-               }
-               IP6_EXTHDR_GET(hbh, struct ip6_hbh *, m, sizeof(struct ip6_hdr),
-                       sizeof(struct ip6_hbh));
-               if (hbh == NULL) {
-                       ip6stat.ip6s_tooshort++;
-                       if_put(ifp);
-                       return;
-               }
-               nxt = hbh->ip6h_nxt;
 
-               /*
-                * accept the packet if a router alert option is included
-                * and we act as an IPv6 router.
-                */
-               if (rtalert != ~0 && ip6_forwarding)
-                       ours = 1;
-       } else
-               nxt = ip6->ip6_nxt;
-
-       /*
-        * Check that the amount of data in the buffers
-        * is as at least much as the IPv6 header would have us expect.
-        * Trim mbufs if longer than we expect.
-        * Drop packet if shorter than we expect.
-        */
-       if (m->m_pkthdr.len - sizeof(struct ip6_hdr) < plen) {
-               ip6stat.ip6s_tooshort++;
-               goto bad;
-       }
-       if (m->m_pkthdr.len > sizeof(struct ip6_hdr) + plen) {
-               if (m->m_len == m->m_pkthdr.len) {
-                       m->m_len = sizeof(struct ip6_hdr) + plen;
-                       m->m_pkthdr.len = sizeof(struct ip6_hdr) + plen;
-               } else
-                       m_adj(m, sizeof(struct ip6_hdr) + plen - 
m->m_pkthdr.len);
+       if (ip6_hbhchcheck(m, &off, &nxt, &ours)) {
+               if_put(ifp);
+               return; /* m have already been freed */
        }
 
+       /* adjust pointer */
+       ip6 = mtod(m, struct ip6_hdr *);
+
        /*
         * Forward if desirable.
         */
@@ -638,6 +575,89 @@ ip6_input(struct mbuf *m)
  bad:
        if_put(ifp);
        m_freem(m);
+}
+
+int
+ip6_hbhchcheck(struct mbuf *m, int *offp, int *nxtp, int *oursp)
+{
+       struct ip6_hdr *ip6;
+       u_int32_t plen, rtalert = ~0;
+
+       ip6 = mtod(m, struct ip6_hdr *);
+
+       /*
+        * Process Hop-by-Hop options header if it's contained.
+        * m may be modified in ip6_hopopts_input().
+        * If a JumboPayload option is included, plen will also be modified.
+        */
+       plen = (u_int32_t)ntohs(ip6->ip6_plen);
+       *offp = sizeof(struct ip6_hdr);
+       if (ip6->ip6_nxt == IPPROTO_HOPOPTS) {
+               struct ip6_hbh *hbh;
+
+               if (ip6_hopopts_input(&plen, &rtalert, &m, offp)) {
+                       return (-1);    /* m have already been freed */
+               }
+
+               /* adjust pointer */
+               ip6 = mtod(m, struct ip6_hdr *);
+
+               /*
+                * if the payload length field is 0 and the next header field
+                * indicates Hop-by-Hop Options header, then a Jumbo Payload
+                * option MUST be included.
+                */
+               if (ip6->ip6_plen == 0 && plen == 0) {
+                       /*
+                        * Note that if a valid jumbo payload option is
+                        * contained, ip6_hopopts_input() must set a valid
+                        * (non-zero) payload length to the variable plen.
+                        */
+                       ip6stat.ip6s_badoptions++;
+                       icmp6_error(m, ICMP6_PARAM_PROB,
+                                   ICMP6_PARAMPROB_HEADER,
+                                   (caddr_t)&ip6->ip6_plen - (caddr_t)ip6);
+                       return (-1);
+               }
+               IP6_EXTHDR_GET(hbh, struct ip6_hbh *, m, sizeof(struct ip6_hdr),
+                       sizeof(struct ip6_hbh));
+               if (hbh == NULL) {
+                       ip6stat.ip6s_tooshort++;
+                       return (-1);
+               }
+               *nxtp = hbh->ip6h_nxt;
+
+               /*
+                * accept the packet if a router alert option is included
+                * and we act as an IPv6 router.
+                */
+               if (rtalert != ~0 && ip6_forwarding)
+                       *oursp = 1;
+       } else
+               *nxtp = ip6->ip6_nxt;
+
+       /*
+        * Check that the amount of data in the buffers
+        * is as at least much as the IPv6 header would have us expect.
+        * Trim mbufs if longer than we expect.
+        * Drop packet if shorter than we expect.
+        */
+       if (m->m_pkthdr.len - sizeof(struct ip6_hdr) < plen) {
+               ip6stat.ip6s_tooshort++;
+               m_freem(m);
+               return (-1);
+       }
+       if (m->m_pkthdr.len > sizeof(struct ip6_hdr) + plen) {
+               if (m->m_len == m->m_pkthdr.len) {
+                       m->m_len = sizeof(struct ip6_hdr) + plen;
+                       m->m_pkthdr.len = sizeof(struct ip6_hdr) + plen;
+               } else {
+                       m_adj(m,
+                           sizeof(struct ip6_hdr) + plen - m->m_pkthdr.len);
+               }
+       }
+
+       return (0);
 }
 
 /* scan packet for RH0 routing header. Mostly stolen from pf.c:pf_test() */

Reply via email to