Now that we have the flow-mod validation with the action/instructions
support we can extend the usage of this functions for the packet-out
validation.

This diff increases the packet-out validation coverage by also doing
instructions and packet truncation checks.

ok?

Index: ofp13.c
===================================================================
RCS file: /cvs/src/usr.sbin/switchd/ofp13.c,v
retrieving revision 1.25
diff -u -p -r1.25 ofp13.c
--- ofp13.c     7 Nov 2016 13:27:11 -0000       1.25
+++ ofp13.c     7 Nov 2016 13:33:34 -0000
@@ -462,10 +462,9 @@ ofp13_validate_packet_out(struct switchd
     struct ofp_header *oh, struct ibuf *ibuf)
 {
        struct ofp_packet_out           *pout;
-       size_t                           len;
-       off_t                            off;
+       size_t                           len, plen, diff;
+       off_t                            off, noff;
        struct ofp_action_header        *ah;
-       struct ofp_action_output        *ao;
 
        off = 0;
        if ((pout = ibuf_seek(ibuf, off, sizeof(*pout))) == NULL) {
@@ -474,36 +473,43 @@ ofp13_validate_packet_out(struct switchd
                return (-1);
        }
 
-       log_debug("\tbuffer %d port %s "
-           "actions length %u",
+       off += sizeof(*pout);
+       len = ntohs(pout->pout_actions_len);
+       log_debug("\tbuffer %d in_port %s actions_len %lu",
            ntohl(pout->pout_buffer_id),
-           print_map(ntohl(pout->pout_in_port), ofp_port_map),
-           ntohs(pout->pout_actions_len));
-       len = ntohl(pout->pout_actions_len);
+           print_map(ntohl(pout->pout_in_port), ofp_port_map), len);
 
-       off += sizeof(*pout);
-       while ((ah = ibuf_seek(ibuf, off, len)) != NULL &&
-           ntohs(ah->ah_len) >= (uint16_t)sizeof(*ah)) {
-               switch (ntohs(ah->ah_type)) {
-               case OFP_ACTION_OUTPUT:
-                       ao = (struct ofp_action_output *)ah;
-                       log_debug("\t\taction type %s length %d "
-                           "port %s max length %d",
-                           print_map(ntohs(ao->ao_type), ofp_action_map),
-                           ntohs(ao->ao_len),
-                           print_map(ntohs(ao->ao_port), ofp_port_map),
-                           ntohs(ao->ao_max_len));
-                       break;
-               default:
-                       log_debug("\t\taction type %s length %d",
-                           print_map(ntohs(ah->ah_type), ofp_action_map),
-                           ntohs(ah->ah_len));
-                       break;
-               }
-               if (pout->pout_buffer_id == (uint32_t)-1)
-                       break;
-               off += ntohs(ah->ah_len);
+parse_next_action:
+       if ((ah = ibuf_seek(ibuf, off, sizeof(*ah))) == NULL)
+               return (-1);
+
+       noff = off;
+       ofp13_validate_action(sc, oh, ibuf, &off, ah);
+
+       diff = off - noff;
+       /* Loop prevention. */
+       if (off < noff || diff == 0)
+               return (-1);
+
+       len -= diff;
+       if (len)
+               goto parse_next_action;
+
+       /* Check for encapsulated packet truncation. */
+       len = ntohs(oh->oh_length) - off;
+       plen = ibuf_length(ibuf) - off;
+
+       if (plen < len) {
+               log_debug("\ttruncated packet %lu < %lu", plen, len);
+
+               /* Buffered packets can be truncated */
+               if (pout->pout_buffer_id != OFP_PKTOUT_NO_BUFFER)
+                       len = plen;
+               else
+                       return (-1);
        }
+       if (ibuf_seek(ibuf, off, len) == NULL)
+               return (-1);
 
        return (0);
 }

Reply via email to