On Thu, Dec 8, 2011 at 2:01 PM, Ben Pfaff <b...@nicira.com> wrote:

> An upcoming commit will add another user.
> ---
>  lib/packets.c          |   25 +++++++++++++++++++++++++
>  lib/packets.h          |    2 ++
>  ofproto/ofproto-dpif.c |   12 ++++--------
>  3 files changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/lib/packets.c b/lib/packets.c
> index 46a44bd..b9f37bb 100644
> --- a/lib/packets.c
> +++ b/lib/packets.c
> @@ -121,6 +121,31 @@ eth_pop_vlan(struct ofpbuf *packet)
>     }
>  }
>
> +/* Converts hex digits in 'hex' to an Ethernet packet in '*packetp'.  The
> + * caller must free '*packetp'.  On success, returns NULL.  On failure,
> returns
> + * an error message and stores NULL in '*packetp'. */
> +const char *
> +eth_from_hex(const char *hex, struct ofpbuf **packetp)
> +{
> +    struct ofpbuf *packet;
> +
> +    packet = *packetp = ofpbuf_new(strlen(hex) / 2);
>
I did not try to apply and test this patch, but it seems that if
strlen(hex) == 1 (either valid or invalid hex digit),
then packet->base==NULL after the previous line.

For example later ofpbuf_delete() could try to do free(NULL)...

+
> +    if (ofpbuf_put_hex(packet, hex, NULL)[0] != '\0') {
> +        ofpbuf_delete(packet);
> +        *packetp = NULL;
> +        return "Trailing garbage in packet data";
> +    }
> +
> +    if (packet->size < ETH_HEADER_LEN) {
> +        ofpbuf_delete(packet);
> +        *packetp = NULL;
> +        return "Packet data too short for Ethernet";
> +    }
> +
> +    return NULL;
> +}
> +
>  /* Given the IP netmask 'netmask', returns the number of bits of the IP
> address
>  * that it specifies, that is, the number of 1-bits in 'netmask'.
>  'netmask'
>  * must be a CIDR netmask (see ip_is_cidr()). */
> diff --git a/lib/packets.h b/lib/packets.h
> index 439a7dd..9e283a5 100644
> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -134,6 +134,8 @@ void compose_benign_packet(struct ofpbuf *, const char
> *tag,
>  void eth_push_vlan(struct ofpbuf *, ovs_be16 tci);
>  void eth_pop_vlan(struct ofpbuf *);
>
> +const char *eth_from_hex(const char *hex, struct ofpbuf **packetp);
> +
>  /* Example:
>  *
>  * uint8_t mac[ETH_ADDR_LEN];
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 88bc620..6bc2a25 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -5632,15 +5632,11 @@ ofproto_unixctl_trace(struct unixctl_conn *conn,
> int argc, const char *argv[],
>         uint16_t in_port = ofp_port_to_odp_port(atoi(in_port_s));
>         ovs_be64 tun_id = htonll(strtoull(tun_id_s, NULL, 0));
>         uint32_t priority = atoi(priority_s);
> +        const char *msg;
>
> -        packet = ofpbuf_new(strlen(packet_s) / 2);
> -        if (ofpbuf_put_hex(packet, packet_s, NULL)[0] != '\0') {
> -            unixctl_command_reply(conn, 501, "Trailing garbage in
> command");
> -            goto exit;
> -        }
> -        if (packet->size < ETH_HEADER_LEN) {
> -            unixctl_command_reply(conn, 501,
> -                                  "Packet data too short for Ethernet");
> +        msg = eth_from_hex(packet_s, &packet);
> +        if (msg) {
> +            unixctl_command_reply(conn, 501, msg);
>             goto exit;
>         }
>
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to