New Defects reported by Coverity Scan for freerangerouting/frr

scan-admin at coverity.com scan-admin at coverity.com
Tue Jan 14 14:58:36 UTC 2025


Hi,

Please find the latest report on new defect(s) introduced to freerangerouting/frr found with Coverity Scan.

128 new defect(s) introduced to freerangerouting/frr found with Coverity Scan.
3 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 20 of 128 defect(s)


** CID 1617478:  Insecure data handling  (INTEGER_OVERFLOW)


________________________________________________________________________________________________________
*** CID 1617478:  Insecure data handling  (INTEGER_OVERFLOW)
/ospfd/ospf_api.c: 642 in new_msg_lsa_change_notify()
636     	len = ntohs(data->length);
637     	if (len > data_maxs)
638     		len = data_maxs;
639     	memcpy(nmsg_data, data, len);
640     	len += sizeof(struct msg_lsa_change_notify) - sizeof(struct lsa_header);
641     
>>>     CID 1617478:  Insecure data handling  (INTEGER_OVERFLOW)
>>>     "len", which might have overflowed, is passed to "msg_new(msgtype, nmsg, seqnum, len)".
642     	return msg_new(msgtype, nmsg, seqnum, len);
643     }
644     
645     struct msg *new_msg_reachable_change(uint32_t seqnum, uint16_t nadd,
646     				     struct in_addr *add, uint16_t nremove,
647     				     struct in_addr *remove)

** CID 1617477:  Data race undermines locking  (LOCK_EVASION)
/bgpd/bgpd.c: 7020 in peer_password_unset()


________________________________________________________________________________________________________
*** CID 1617477:  Data race undermines locking  (LOCK_EVASION)
/bgpd/bgpd.c: 7020 in peer_password_unset()
7014     	for (ALL_LIST_ELEMENTS(peer->group->peer, node, nnode, member)) {
7015     		/* Skip peers with overridden configuration. */
7016     		if (CHECK_FLAG(member->flags_override, PEER_FLAG_PASSWORD))
7017     			continue;
7018     
7019     		/* Remove flag and configuration on peer-group member. */
>>>     CID 1617477:  Data race undermines locking  (LOCK_EVASION)
>>>     Thread1 sets "flags" to a new value. Now the two threads have an inconsistent view of "flags" and updates to fields correlated with "flags" may be lost.
7020     		UNSET_FLAG(member->flags, PEER_FLAG_PASSWORD);
7021     		XFREE(MTYPE_PEER_PASSWORD, member->password);
7022     
7023     		/* Send notification or reset peer depending on state. */
7024     		if (!peer_notify_config_change(member->connection))
7025     			bgp_session_reset(member);

** CID 1617476:  Insecure data handling  (INTEGER_OVERFLOW)


________________________________________________________________________________________________________
*** CID 1617476:  Insecure data handling  (INTEGER_OVERFLOW)
/ospfd/ospf_api.c: 669 in new_msg_reachable_change()
663     		memcpy(&nmsg->router_ids[nadd], remove, nremove * insz);
664     
665     	nmsg->nadd = htons(nadd);
666     	nmsg->nremove = htons(nremove);
667     	len = sizeof(*nmsg) + insz * (nadd + nremove);
668     
>>>     CID 1617476:  Insecure data handling  (INTEGER_OVERFLOW)
>>>     "len", which might have overflowed, is passed to "msg_new(18, nmsg, seqnum, len)".
669     	return msg_new(MSG_REACHABLE_CHANGE, nmsg, seqnum, len);
670     }
671     
672     struct msg *new_msg_router_id_change(uint32_t seqnum, struct in_addr router_id)
673     {
674     	struct msg_router_id_change rmsg = {.router_id = router_id};

** CID 1617475:  Program hangs  (LOCK)
/zebra/zapi_msg.c: 1443 in zread_fec_register()


________________________________________________________________________________________________________
*** CID 1617475:  Program hangs  (LOCK)
/zebra/zapi_msg.c: 1443 in zread_fec_register()
1437     			l += 4;
1438     		} else if (flags & ZEBRA_FEC_REGISTER_LABEL_INDEX) {
1439     			STREAM_GETL(s, label_index);
1440     			l += 4;
1441     		}
1442     
>>>     CID 1617475:  Program hangs  (LOCK)
>>>     "zebra_mpls_fec_register" locks "client->obuf_mtx" while it is locked.
1443     		zebra_mpls_fec_register(zvrf, &p, label, label_index, client);
1444     	}
1445     
1446     stream_failure:
1447     	return;
1448     }

** CID 1617474:    (LOCK)
/bgpd/bgpd.c: 8684 in bgp_pthreads_run()
/bgpd/bgpd.c: 8684 in bgp_pthreads_run()


________________________________________________________________________________________________________
*** CID 1617474:    (LOCK)
/bgpd/bgpd.c: 8684 in bgp_pthreads_run()
8678     	frr_pthread_run(bgp_pth_io, NULL);
8679     	frr_pthread_run(bgp_pth_ka, NULL);
8680     
8681     	/* Wait until threads are ready. */
8682     	frr_pthread_wait_running(bgp_pth_io);
8683     	frr_pthread_wait_running(bgp_pth_ka);
>>>     CID 1617474:    (LOCK)
>>>     Returning without unlocking "bgp_pth_io->running_cond_mtx".
8684     }
8685     
8686     void bgp_pthreads_finish(void)
8687     {
8688     	frr_pthread_stop_all();
8689     }
/bgpd/bgpd.c: 8684 in bgp_pthreads_run()
8678     	frr_pthread_run(bgp_pth_io, NULL);
8679     	frr_pthread_run(bgp_pth_ka, NULL);
8680     
8681     	/* Wait until threads are ready. */
8682     	frr_pthread_wait_running(bgp_pth_io);
8683     	frr_pthread_wait_running(bgp_pth_ka);
>>>     CID 1617474:    (LOCK)
>>>     Returning without unlocking "bgp_pth_ka->running_cond_mtx".
8684     }
8685     
8686     void bgp_pthreads_finish(void)
8687     {
8688     	frr_pthread_stop_all();
8689     }

** CID 1617473:  Program hangs  (LOCK)
/bgpd/rfapi/rfapi_vty.c: 976 in rfapiShowVncQueries()


________________________________________________________________________________________________________
*** CID 1617473:  Program hangs  (LOCK)
/bgpd/rfapi/rfapi_vty.c: 976 in rfapiShowVncQueries()
970     
971     					fp(out, "%-15s %-15s", buf_vn, buf_un);
972     					printedquerier = 1;
973     				} else
974     					fp(out, "%-15s %-15s", "", "");
975     				buf_remain[0] = 0;
>>>     CID 1617473:  Program hangs  (LOCK)
>>>     "event_timer_remain_second" locks "mon_eth->timer->mtx" while it is locked.
976     				rfapiFormatSeconds(event_timer_remain_second(
977     							   mon_eth->timer),
978     						   buf_remain, BUFSIZ);
979     				fp(out, " %-17s %10d %-10s\n",
980     				   rfapi_ntop(pfx_mac.family, &pfx_mac.u.prefix,
981     					      buf_pfx, BUFSIZ),

** CID 1617472:  Program hangs  (LOCK)
/bgpd/bgp_keepalives.c: 227 in bgp_keepalives_start()


________________________________________________________________________________________________________
*** CID 1617472:  Program hangs  (LOCK)
/bgpd/bgp_keepalives.c: 227 in bgp_keepalives_start()
221     		TIMEVAL_TO_TIMESPEC(&next_update, &next_update_ts);
222     	}
223     
224     	/* clean up */
225     	pthread_cleanup_pop(1);
226     
>>>     CID 1617472:  Program hangs  (LOCK)
>>>     Returning without unlocking "fpt->running_cond_mtx".
227     	return NULL;
228     }
229     
230     /* --- thread external functions ------------------------------------------- */
231     
232     void bgp_keepalives_on(struct peer_connection *connection)

** CID 1617471:  Program hangs  (LOCK)
/ospfd/ospf_lsa.c: 3210 in ospf_maxage_lsa_remover()


________________________________________________________________________________________________________
*** CID 1617471:  Program hangs  (LOCK)
/ospfd/ospf_lsa.c: 3210 in ospf_maxage_lsa_remover()
3204     			if (lsa->retransmit_counter > 0) {
3205     				reschedule = 1;
3206     				continue;
3207     			}
3208     
3209     			/* TODO: maybe convert this function to a work-queue */
>>>     CID 1617471:  Program hangs  (LOCK)
>>>     "event_should_yield" locks "event->mtx" while it is locked.
3210     			if (event_should_yield(event)) {
3211     				OSPF_TIMER_ON(ospf->t_maxage,
3212     					      ospf_maxage_lsa_remover, 0);
3213     				route_unlock_node(
3214     					rn); /* route_top/route_next */
3215     				return;

** CID 1617470:  Insecure data handling  (INTEGER_OVERFLOW)


________________________________________________________________________________________________________
*** CID 1617470:  Insecure data handling  (INTEGER_OVERFLOW)
/ospfd/ospf_api.c: 517 in new_msg_originate_request()
511     	if (omsglen > data_maxs)
512     		omsglen = data_maxs;
513     	memcpy(omsg_data, data, omsglen);
514     	omsglen += sizeof(struct msg_originate_request)
515     		   - sizeof(struct lsa_header);
516     
>>>     CID 1617470:  Insecure data handling  (INTEGER_OVERFLOW)
>>>     "omsglen", which might have overflowed, is passed to "msg_new(5, omsg, seqnum, omsglen)".
517     	return msg_new(MSG_ORIGINATE_REQUEST, omsg, seqnum, omsglen);
518     }
519     
520     struct msg *new_msg_delete_request(uint32_t seqnum, struct in_addr addr,
521     				   uint8_t lsa_type, uint8_t opaque_type,
522     				   uint32_t opaque_id, uint8_t flags)

** CID 1617469:  Program hangs  (ORDER_REVERSAL)
/bgpd/bgp_fsm.c: 1504 in bgp_stop()


________________________________________________________________________________________________________
*** CID 1617469:  Program hangs  (ORDER_REVERSAL)
/bgpd/bgp_fsm.c: 1504 in bgp_stop()
1498     	EVENT_OFF(connection->t_connect);
1499     	EVENT_OFF(connection->t_holdtime);
1500     	EVENT_OFF(connection->t_routeadv);
1501     	EVENT_OFF(connection->t_delayopen);
1502     
1503     	/* Clear input and output buffer.  */
>>>     CID 1617469:  Program hangs  (ORDER_REVERSAL)
>>>     Calling "_frr_mtx_lock" acquires lock "peer_connection.io_mtx" while holding lock "event_loop.mtx" (count: 5 / 51).
1504     	frr_with_mutex (&connection->io_mtx) {
1505     		if (connection->ibuf)
1506     			stream_fifo_clean(connection->ibuf);
1507     		if (connection->obuf)
1508     			stream_fifo_clean(connection->obuf);
1509     

** CID 1617468:  Integer handling issues  (INTEGER_OVERFLOW)
/lib/bitfield.h: 204 in bf_find_next_clear_bit_wrap()


________________________________________________________________________________________________________
*** CID 1617468:  Integer handling issues  (INTEGER_OVERFLOW)
/lib/bitfield.h: 204 in bf_find_next_clear_bit_wrap()
198     
199     	/*
200     	 * start looking for a clear bit at the start of the bitfield and
201     	 * stop when we reach start_index
202     	 */
203     	scanbits = WORD_SIZE;
>>>     CID 1617468:  Integer handling issues  (INTEGER_OVERFLOW)
>>>     Expression "start_index - 1U", where "start_index" is known to be equal to 0, underflows the type of "start_index - 1U", which is type "unsigned int".
204     	index_max = bf_index(start_index - 1);
205     	for (i = 0; i <= index_max; ++i) {
206     		if (i == index_max)
207     			scanbits = ((start_index - 1) % WORD_SIZE) + 1;
208     		for (offset = start_bit; offset < scanbits; ++offset) {
209     			if (!((v->data[i] >> offset) & 1))

** CID 1617467:  Program hangs  (LOCK)
/pathd/path_pcep_controller.c: 200 in pcep_ctrl_initialize()


________________________________________________________________________________________________________
*** CID 1617467:  Program hangs  (LOCK)
/pathd/path_pcep_controller.c: 200 in pcep_ctrl_initialize()
194     	ctrl_state->pcc_opts->addr.ipa_type = IPADDR_NONE;
195     	ctrl_state->pcc_opts->port = PCEP_DEFAULT_PORT;
196     
197     	/* Keep the state reference for events */
198     	set_ctrl_state(*fpt, ctrl_state);
199     
>>>     CID 1617467:  Program hangs  (LOCK)
>>>     Returning without unlocking "(*fpt)->running_cond_mtx".
200     	return ret;
201     }
202     
203     int pcep_ctrl_finalize(struct frr_pthread **fpt)
204     {
205     	assert(fpt != NULL);

** CID 1617466:  Concurrent data access violations  (MISSING_LOCK)
/zebra/zebra_fpm.c: 1760 in zfpm_show_stats()


________________________________________________________________________________________________________
*** CID 1617466:  Concurrent data access violations  (MISSING_LOCK)
/zebra/zebra_fpm.c: 1760 in zfpm_show_stats()
1754     	ZFPM_SHOW_STAT(nop_deletes_skipped);
1755     	ZFPM_SHOW_STAT(route_adds);
1756     	ZFPM_SHOW_STAT(route_dels);
1757     	ZFPM_SHOW_STAT(updates_triggered);
1758     	ZFPM_SHOW_STAT(redundant_triggers);
1759     	ZFPM_SHOW_STAT(dests_del_after_update);
>>>     CID 1617466:  Concurrent data access violations  (MISSING_LOCK)
>>>     Accessing "zfpm_g->last_ivl_stats.t_conn_down_starts" without holding lock "event_loop.mtx". Elsewhere, "zfpm_stats.t_conn_down_starts" is written to with "event_loop.mtx" held 1 out of 1 times.
1760     	ZFPM_SHOW_STAT(t_conn_down_starts);
1761     	ZFPM_SHOW_STAT(t_conn_down_dests_processed);
1762     	ZFPM_SHOW_STAT(t_conn_down_yields);
1763     	ZFPM_SHOW_STAT(t_conn_down_finishes);
1764     	ZFPM_SHOW_STAT(t_conn_up_starts);
1765     	ZFPM_SHOW_STAT(t_conn_up_dests_processed);

** CID 1617465:  Data race undermines locking  (LOCK_EVASION)
/zebra/zserv.c: 721 in zserv_close_client()


________________________________________________________________________________________________________
*** CID 1617465:  Data race undermines locking  (LOCK_EVASION)
/zebra/zserv.c: 721 in zserv_close_client()
715     		event_cancel_event(zrouter.master, client);
716     		EVENT_OFF(client->t_cleanup);
717     		EVENT_OFF(client->t_process);
718     
719     		/* destroy pthread */
720     		frr_pthread_destroy(client->pthread);
>>>     CID 1617465:  Data race undermines locking  (LOCK_EVASION)
>>>     Thread1 sets "pthread" to a new value. Now the two threads have an inconsistent view of "pthread" and updates to fields of "pthread" or fields correlated with "pthread" may be lost.
721     		client->pthread = NULL;
722     	}
723     
724     	/*
725     	 * Final check in case the client struct is in use in another
726     	 * pthread: if not in-use, continue and free the client

** CID 1617464:  Concurrent data access violations  (MISSING_LOCK)
/zebra/zebra_fpm.c: 1764 in zfpm_show_stats()


________________________________________________________________________________________________________
*** CID 1617464:  Concurrent data access violations  (MISSING_LOCK)
/zebra/zebra_fpm.c: 1764 in zfpm_show_stats()
1758     	ZFPM_SHOW_STAT(redundant_triggers);
1759     	ZFPM_SHOW_STAT(dests_del_after_update);
1760     	ZFPM_SHOW_STAT(t_conn_down_starts);
1761     	ZFPM_SHOW_STAT(t_conn_down_dests_processed);
1762     	ZFPM_SHOW_STAT(t_conn_down_yields);
1763     	ZFPM_SHOW_STAT(t_conn_down_finishes);
>>>     CID 1617464:  Concurrent data access violations  (MISSING_LOCK)
>>>     Accessing "zfpm_g->last_ivl_stats.t_conn_up_starts" without holding lock "event_loop.mtx". Elsewhere, "zfpm_stats.t_conn_up_starts" is written to with "event_loop.mtx" held 1 out of 1 times.
1764     	ZFPM_SHOW_STAT(t_conn_up_starts);
1765     	ZFPM_SHOW_STAT(t_conn_up_dests_processed);
1766     	ZFPM_SHOW_STAT(t_conn_up_yields);
1767     	ZFPM_SHOW_STAT(t_conn_up_aborts);
1768     	ZFPM_SHOW_STAT(t_conn_up_finishes);
1769     

** CID 1617463:    (INTEGER_OVERFLOW)
/lib/ptm_lib.c: 320 in _ptm_lib_read_ptm_socket()
/lib/ptm_lib.c: 307 in _ptm_lib_read_ptm_socket()


________________________________________________________________________________________________________
*** CID 1617463:    (INTEGER_OVERFLOW)
/lib/ptm_lib.c: 320 in _ptm_lib_read_ptm_socket()
314     			if (retries++ < 2) {
315     				usleep(10000);
316     				continue;
317     			}
318     			DLOG("max retries - recv error(%d - %s) bytes read %d (%d)\n", errno,
319     			     strerror(errno), bytes_read, len);
>>>     CID 1617463:    (INTEGER_OVERFLOW)
>>>     "bytes_read", which might have overflowed, is returned from the function.
320     			return (bytes_read);
321     		} else {
322     			bytes_read += rc;
323     		}
324     	}
325     
/lib/ptm_lib.c: 307 in _ptm_lib_read_ptm_socket()
301     static int _ptm_lib_read_ptm_socket(int fd, char *buf, int len)
302     {
303     	int retries = 0, rc;
304     	int bytes_read = 0;
305     
306     	while (bytes_read != len) {
>>>     CID 1617463:    (INTEGER_OVERFLOW)
>>>     "len - bytes_read", which might have underflowed, is passed to "recv(fd, (void *)(buf + bytes_read), len - bytes_read, MSG_DONTWAIT)".
307     		rc = recv(fd, (void *)(buf + bytes_read), (len - bytes_read),
308     			  MSG_DONTWAIT);
309     		if (rc < 0 && (errno != EAGAIN) && (errno != EWOULDBLOCK)) {
310     			ERRLOG("fatal recv error(%s), closing connection, rc %d\n", strerror(errno),
311     			       rc);
312     			return (rc);

** CID 1617462:  Program hangs  (ORDER_REVERSAL)


________________________________________________________________________________________________________
*** CID 1617462:  Program hangs  (ORDER_REVERSAL)
/zebra/zebra_pw.c: 239 in zebra_pw_install_failure()
233     
234     	/* schedule to retry later */
235     	EVENT_OFF(pw->install_retry_timer);
236     	event_add_timer(zrouter.master, zebra_pw_install_retry, pw,
237     			PW_INSTALL_RETRY_INTERVAL, &pw->install_retry_timer);
238     
>>>     CID 1617462:  Program hangs  (ORDER_REVERSAL)
>>>     Calling "zebra_pw_update_status" acquires lock "zserv.obuf_mtx" while holding lock "event_loop.mtx" (count: 1 / 2).
239     	zebra_pw_update_status(pw, pwstatus);
240     }
241     
242     static void zebra_pw_install_retry(struct event *thread)
243     {
244     	struct zebra_pw *pw = EVENT_ARG(thread);

** CID 1617461:  Memory - corruptions  (USE_AFTER_FREE)
/bgpd/bgp_route.c: 9317 in bgp_redistribute_add()


________________________________________________________________________________________________________
*** CID 1617461:  Memory - corruptions  (USE_AFTER_FREE)
/bgpd/bgp_route.c: 9317 in bgp_redistribute_add()
9311     				bgp->peer_self, new_attr, bn);
9312     		SET_FLAG(new->flags, BGP_PATH_VALID);
9313     
9314     		bgp_aggregate_increment(bgp, p, new, afi, SAFI_UNICAST);
9315     		bgp_path_info_add(bn, new);
9316     		bgp_dest_unlock_node(bn);
>>>     CID 1617461:  Memory - corruptions  (USE_AFTER_FREE)
>>>     Dereferencing freed pointer "bn".
9317     		SET_FLAG(bn->flags, BGP_NODE_FIB_INSTALLED);
9318     		bgp_process(bgp, bn, new, afi, SAFI_UNICAST);
9319     
9320     		if ((bgp->inst_type == BGP_INSTANCE_TYPE_VRF)
9321     		    || (bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) {
9322     

** CID 1617460:  Program hangs  (ORDER_REVERSAL)


________________________________________________________________________________________________________
*** CID 1617460:  Program hangs  (ORDER_REVERSAL)
/bgpd/bgpd.c: 1291 in peer_free()
1285     
1286     	if (peer->change_local_as_pretty)
1287     		XFREE(MTYPE_BGP_NAME, peer->change_local_as_pretty);
1288     	if (peer->as_pretty)
1289     		XFREE(MTYPE_BGP_NAME, peer->as_pretty);
1290     
>>>     CID 1617460:  Program hangs  (ORDER_REVERSAL)
>>>     Calling "bgp_peer_connection_free" acquires lock "peer_connection.io_mtx" while holding lock "event_loop.mtx" (count: 5 / 51).
1291     	bgp_peer_connection_free(&peer->connection);
1292     
1293     	bgp_unlock(peer->bgp);
1294     
1295     	stream_free(peer->last_reset_cause);
1296     

** CID 1617459:  Data race undermines locking  (LOCK_EVASION)
/ospf6d/ospf6_network.c: 89 in ospf6_serv_sock()


________________________________________________________________________________________________________
*** CID 1617459:  Data race undermines locking  (LOCK_EVASION)
/ospf6d/ospf6_network.c: 89 in ospf6_serv_sock()
83     	ospf6_set_reuseaddr();
84     #endif /*1*/
85     	ospf6_reset_mcastloop(ospf6_sock);
86     	ospf6_set_pktinfo(ospf6_sock);
87     	ospf6_set_transport_class(ospf6_sock);
88     
>>>     CID 1617459:  Data race undermines locking  (LOCK_EVASION)
>>>     Thread1 sets "fd" to a new value. Now the two threads have an inconsistent view of "fd" and updates to fields correlated with "fd" may be lost.
89     	ospf6->fd = ospf6_sock;
90     	/* setup global in6_addr, allspf6 and alldr6 for later use */
91     	inet_pton(AF_INET6, ALLSPFROUTERS6, &allspfrouters6);
92     	inet_pton(AF_INET6, ALLDROUTERS6, &alldrouters6);
93     
94     	return 0;


________________________________________________________________________________________________________
To view the defects in Coverity Scan visit, https://scan.coverity.com/projects/freerangerouting-frr?tab=overview




More information about the dev mailing list