Comments (10)
I utilized Windbg and ApplicationVerifier for debugging. Quite often it pointed at generate_report function, specifically the construct_sdes_chunk.
After further investigation, I found out that there is an overflow in the generate_report function. This does not happen always, but after the construct_sdes_chunk call, the value of write_ptr is greater than compound_packet_size.
I put printouts in the generate_report function. They are pretty self explanatory. The last one is inside construct_sdes_chunk, right before memcpy.
The program crashes on that memcpy line with Windbg and ApplicationVerifier. Otherwise it can run for hours before crashing into a heap corruption.
from uvgrtp.
Hi.
I'm sorry to hear that you have encountered a bug. Are you setting the SDES items by calling send_sdes_packet()-function? From your description, the problem seems to be that new SDES items appear between the time the compound_packet_size is calculated and the packet itself is constructed. There is a mutex called packet_mutex_ which should prevent all modifications to packet data while they are being sent, so I'm unable to figure out the cause for this error at this time.
BR, Joni
from uvgrtp.
No, I am not calling send_sdes_packet()
from uvgrtp.
Are you recreating or restarting the stream? I was just wondering if there is something wrong with the stopping of the RTCP, which could explain this behavior.
BR, Joni
from uvgrtp.
I am not sure, I have observed a crash after running a test for a day, and after running for a minute. I cannot say whether stream recreation has something to do with it
from uvgrtp.
@jrsnen I have found the problem. I don't know how to fix it, hopefully you have some ideas
In uvgrtp::rtcp::generate_report() on line 1612 we calculate the reports var:
uint8_t reports = 0;
for (auto& p : participants_)
{
if (p.second->stats.received_rtp_packet)
{
++reports;
}
}
a little later it is used to allocate memory for packet array:
uint32_t compound_packet_size = size_of_compound_packet(reports, sr_packet, rr_packet, sdes_packet, app_packets_size, bye_packet);
Later in this function, when the memory is already allocated, we check participant stats again to populate the array:
for (auto& p : participants_)
{
// only add report blocks if we have received data from them
if (p.second->stats.received_rtp_packet)
{
uint32_t dropped_packets = p.second->stats.dropped_pkts;
// TODO: Fraction should be the number of packets lost compared to number of packets expected (see fraction lost in RFC 3550)
// see https://datatracker.ietf.org/doc/html/rfc3550#appendix-A.3
//uint8_t fraction = dropped_packets ? p.second->stats.received_bytes / dropped_packets : 0;
uint8_t fraction = 0; // disabled, because it was incorrect
uint64_t diff = (u_long)uvgrtp::clock::hrc::diff_now(p.second->stats.sr_ts);
uint32_t dlrs = (uint32_t)uvgrtp::clock::ms_to_jiffies(diff);
/* calculate delay of last SR only if SR has been received at least once */
if (p.second->stats.lsr == 0)
{
dlrs = 0;
}
construct_report_block(frame, write_ptr, p.first, fraction, dropped_packets,
p.second->stats.cycles, p.second->stats.max_seq, (uint32_t)p.second->stats.jitter,
p.second->stats.lsr, dlrs);
// we only send reports if there is something to report since last report
p.second->stats.received_rtp_packet = false;
}
}
The problem is that the stats of participants can be modified from another thread. So, when allocating the array, received_rtp_packet is false, but it can happen so that when we are populating the array received_rtp_packet becomes true, and the array overflows.
from uvgrtp.
@wowaser thanks, based on your description I think I know what is the problem.
The problem seems to be that while modifying the SDES structures is guarded by a mutex, adding participants is not guarded by any mutexes and that leads to this crash. This problem seems to exist also in other functions in RTCP class.
This crash and other possible problems would in my option be best solved by adding a new mutex and locking it around every usage of participants_-variable and in generate_report()-function the locked section should be around both uses so that no modification can take place in between.
BR, Joni
from uvgrtp.
@jrsnen Should I add a pull request where I place a couple mutexes around, or do you have more major changes in mind?
For example removing the error-prone array initialization in favor of a dynamic data structure (like a vector) and fetching the size after it's populated?
from uvgrtp.
@wowaser A PR would be great! I guess the smart way to do this is to the reverse of what reception does using the same structures, but this does not seem worth the effort at this point.
from uvgrtp.
Fixed by #188
from uvgrtp.
Related Issues (20)
- Multiplexing packets based on protocol HOT 3
- Streaming 4K H264 video through Wireguard VPN HOT 9
- Compilation on Nanopi board HOT 3
- RTCP interval issues HOT 2
- A mistake for APP packet payload copying HOT 1
- Python API HOT 1
- Failed to flush the message queue HOT 15
- RTP header extension HOT 2
- uint8 overflow in a test HOT 2
- H264 Failed to flush the message queue HOT 16
- H265 Failed to flush the message queue HOT 3
- Streaming H264 video HOT 2
- Visual Studio Library Linking Documentation HOT 2
- Not Receiving RTP Packets from FFmpeg HOT 5
- H26x: Aggregation causes NAL units to be sent in different order HOT 5
- H26x incorrectly detected start code if preceded by 0x1 HOT 1
- H26x 00 01 00 detected as start code at certain alignments HOT 3
- bug in reception_flow.cc HOT 3
- error in uvgrtp::formats::h26x::packet_handler HOT 2
- [Android] Library fails to compile (at least for old SDK level 21) HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from uvgrtp.