** CID 477972: Concurrent data access violations (MISSING_LOCK)

[alfredh]

New coverity scan warnings:

Hi,

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

2 new defect(s) introduced to alfredh/re found with Coverity Scan.
2 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 2 of 2 defect(s)


** CID 477972:  Concurrent data access violations  (MISSING_LOCK)
/src/rtp/sess.c: 470 in sdes_encode_handler()


________________________________________________________________________________________________________
*** CID 477972:  Concurrent data access violations  (MISSING_LOCK)
/src/rtp/sess.c: 470 in sdes_encode_handler()
464     
465     
466     static int sdes_encode_handler(struct mbuf *mb, void *arg)
467     {
468     	struct rtcp_sess *sess = arg;
469     
>>>     CID 477972:  Concurrent data access violations  (MISSING_LOCK)
>>>     Accessing "sess->cname" without holding lock "rtcp_sess.lock". Elsewhere, "rtcp_sess.cname" is written to with "rtcp_sess.lock" held 2 out of 4 times (1 of these accesses strongly imply that it is necessary).
470     	return rtcp_sdes_encode(mb, rtp_sess_ssrc(sess->rs), 1,
471     				RTCP_SDES_CNAME, sess->cname);
472     }
473     
474     
475     static int mk_sdes(struct rtcp_sess *sess, struct mbuf *mb)

** CID 477971:  Concurrent data access violations  (MISSING_LOCK)
/src/rtp/sess.c: 80 in sess_destructor()


________________________________________________________________________________________________________
*** CID 477971:  Concurrent data access violations  (MISSING_LOCK)
/src/rtp/sess.c: 80 in sess_destructor()
74     
75     	if (sess->cname)
76     		(void)send_bye_packet(sess);
77     
78     	tmr_cancel(&sess->tmr);
79     
>>>     CID 477971:  Concurrent data access violations  (MISSING_LOCK)
>>>     Accessing "sess->cname" without holding lock "rtcp_sess.lock". Elsewhere, "rtcp_sess.cname" is written to with "rtcp_sess.lock" held 2 out of 4 times (1 of these accesses strongly imply that it is necessary).
80     	mem_deref(sess->cname);
81     	hash_flush(sess->members);
82     	mem_deref(sess->members);
83     	mem_deref(sess->lock);
84     }
85     

[cspiel1]

I will have a look on this.

[cspiel1]

I opened this PR: #1058

Do we have to start the coverity workflow manually?

[sreimers]

Do we have to start the coverity workflow manually?

Yes only the coverity branch and a release is checked, because coverity has daily and weekly limits how often it can be run. So I merge manually from time to time.

[sreimers]

The sdes_encode_handler issue is fixed. Unsure if the sess_destructor() violation is a real issue. The missing_lock check is very primitive and checks only probabilities.

[cspiel1]

The warning is very concrete. But it makes not much sense to lock the mem_deref(sess->cname) just before the mutex and the whole sess object is destroyed. The application has to ensure that the object is not destroyed before all other threads are finished. Do you think that a mutex lock in the destructor helps?