[ovs-dev] [error reporting 6/6] ofproto: Reject invalid input ports in OFPT_PACKET_OUT requests.

[Ethan Jackson]

Looks good.

Ethan

On Thu, Sep 8, 2011 at 12:36, Ben Pfaff <blp at nicira.com> wrote:
> Some invalid ports (those above the maximum port number supported by the
> datapath, including OpenFlow reserved ports that are not translated by OVS
> into some other number) will be rejected by the datapath.  It's better to
> catch these early and send back an appropriate OpenFlow error code, rather
> than to just get EINVAL from the kernel and have to guess at the problem.
>
> Reported-by: Aaron Rosen <arosen at clemson.edu>
> ---
>  include/openflow/nicira-ext.h |    3 +++
>  ofproto/ofproto-dpif.c        |    4 ++++
>  ofproto/ofproto.c             |   13 ++++++++++++-
>  3 files changed, 19 insertions(+), 1 deletions(-)
>
> diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
> index 890e974..0d68fb3 100644
> --- a/include/openflow/nicira-ext.h
> +++ b/include/openflow/nicira-ext.h
> @@ -110,6 +110,9 @@ enum nx_bad_request_code {
>
>     /* NXT_ROLE_REQUEST specified an invalid role. */
>     NXBRC_BAD_ROLE = 0x201,
> +
> +    /* The in_port in an ofp_packet_out request is invalid. */
> +    NXBRC_BAD_IN_PORT = 0x202
>  };
>
>  /* Additional "code" values for OFPET_FLOW_MOD_FAILED. */
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index f09c230..54dd7a2 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -3920,6 +3920,10 @@ packet_out(struct ofproto *ofproto_, struct ofpbuf *packet,
>     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
>     int error;
>
> +    if (flow->in_port >= ofproto->max_ports && flow->in_port < OFPP_MAX) {
> +        return ofp_mkerr_nicira(OFPET_BAD_REQUEST, NXBRC_BAD_IN_PORT);
> +    }
> +
>     error = validate_actions(ofp_actions, n_ofp_actions, flow,
>                              ofproto->max_ports);
>     if (!error) {
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 156dc90..355e46d 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -1575,6 +1575,7 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh)
>     struct ofpbuf request;
>     struct flow flow;
>     size_t n_ofp_actions;
> +    uint16_t in_port;
>     int error;
>
>     COVERAGE_INC(ofproto_packet_out);
> @@ -1608,8 +1609,18 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh)
>         buffer = NULL;
>     }
>
> +    /* Get in_port and partially validate it.
> +     *
> +     * We don't know what range of ports the ofproto actually implements, but
> +     * we do know that only certain reserved ports (numbered OFPP_MAX and
> +     * above) are valid. */
> +    in_port = ntohs(opo->in_port);
> +    if (in_port >= OFPP_MAX && in_port != OFPP_LOCAL && in_port != OFPP_NONE) {
> +        return ofp_mkerr_nicira(OFPET_BAD_REQUEST, NXBRC_BAD_IN_PORT);
> +    }
> +
>     /* Send out packet. */
> -    flow_extract(&payload, 0, ntohs(opo->in_port), &flow);
> +    flow_extract(&payload, 0, in_port, &flow);
>     error = p->ofproto_class->packet_out(p, &payload, &flow,
>                                          ofp_actions, n_ofp_actions);
>     ofpbuf_delete(buffer);
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>