|SUCCESS| pw159215 [PATCH v5 04/17] graph: avoid overflowing comment buffer

[0-day Robot <robot@bytheb.org>]

Test-Label: 0day Robot - Claude SONNET
Test-Status: SUCCESS
http://dpdk.org/patch/159215

_AI Review Comments Available_

git sha: [commit SHA not available in patch format]
Author: Stephen Hemminger <stephen@networkplumber.org>
Subject: [PATCH v5 04/17] graph: avoid overflowing comment buffer

The library builds a pcapng comment to put on each packet but the buffer 
is not big enough for the largest possible graph node name, etc. Change 
to use asprintf() to allow any string length. Note, if asprintf() fails 
it is better to still log the packet without comment so no data is lost.

> diff --git a/lib/graph/graph_pcap.c b/lib/graph/graph_pcap.c
> index 08dcda0d28..602be79ced 100644
> --- a/lib/graph/graph_pcap.c
> +++ b/lib/graph/graph_pcap.c
> @@ -194,7 +194,7 @@ graph_pcap_dispatch(struct rte_graph *graph,
>  			      uint16_t nb_objs)
>  {
>  	struct rte_mbuf *mbuf_clones[RTE_GRAPH_BURST_SIZE];
> -	char buffer[GRAPH_PCAP_BUF_SZ];
> +	char *comment = NULL;
>  	uint64_t i, num_packets;
>  	struct rte_mbuf *mbuf;
>  	ssize_t len;
> @@ -207,19 +207,22 @@ graph_pcap_dispatch(struct rte_graph *graph,
>  	if (num_packets > nb_objs)
>  		num_packets = nb_objs;
>  
> -	snprintf(buffer, GRAPH_PCAP_BUF_SZ, "%s: %s", graph->name, node->name);
> +	/* put a comment on all these packets */
> +	if (asprintf(&comment, "%s: %s", graph->name, node->name) < 0)
> +		graph_err("asprintf for comment failed.");

Will comment remain NULL if asprintf() fails? The function continues to 
use comment for packet capture even after asprintf() failure, passing 
NULL to rte_pcapng_copy().

>  
>  	for (i = 0; i < num_packets; i++) {
>  		struct rte_mbuf *mc;
>  		mbuf = (struct rte_mbuf *)objs[i];
>  
>  		mc = rte_pcapng_copy(mbuf->port, 0, mbuf, pkt_mp, mbuf->pkt_len,
> -				     0, buffer);
> +				     0, comment);

What happens when rte_pcapng_copy() receives comment as NULL? Does this 
function handle NULL comment parameters safely?

>  		if (mc == NULL)
>  			break;
>  
>  		mbuf_clones[i] = mc;
>  	}
> +	free(comment);

Does free(NULL) behave correctly when asprintf() failed and comment 
remains NULL?

>  
>  	/* write it to capture file */
>  	len = rte_pcapng_write_packets(pcapng_fd, mbuf_clones, i);