Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compression overflow bug #2632

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

john08burke
Copy link
Contributor

Fixes #2602

When rebuilding headers for compaction, we cannot shortcut non-compacted headers at assembly time by using raw hdr_field.len for a few reasons: (1) calculated compact buffer len is based on hdr_field.body not hdr_field.len, (2) hdr_field.body is sanitized and may have adjusted len.
@github-actions
Copy link

Any updates here? No progress has been made in the last 30 days, marking as stale.

@github-actions github-actions bot added the stale label Oct 22, 2021
@john08burke
Copy link
Contributor Author

keepalive

@stale stale bot removed the stale label Oct 22, 2021
@github-actions
Copy link

Any updates here? No progress has been made in the last 30 days, marking as stale.

@github-actions github-actions bot added the stale label Nov 22, 2021
@john08burke
Copy link
Contributor Author

Keepalive

@stale stale bot removed the stale label Nov 25, 2021
@stale
Copy link

stale bot commented Jan 9, 2022

Any updates here? No progress has been made in the last 15 days, marking as stale. Will close this issue if no further updates are made in the next 30 days.

@stale stale bot added the stale label Jan 9, 2022
@liviuchircu liviuchircu self-assigned this Jan 10, 2022
@stale stale bot removed the stale label Jan 10, 2022
@liviuchircu liviuchircu added this to the 3.1.8 milestone Jan 10, 2022
@liviuchircu liviuchircu self-requested a review January 10, 2022 10:56
&new_buf.s,
hdr_mask[i]->name.s,
/* Possible siblings. No CRLF yet */
hdr_mask[i]->len - CRLF_LEN,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @john08burke , I do not have too much expertise over this module, but something tells me that this is not the right place for the fix. How the len of the hdr is calculated here looks ok to me. But what I suspect is that the msg_total_len is badly computed. Do you still have a core file with this crash? or can you reproduce it ? Maybe we can identify where the mismatch between msg_total_len calculation and the writing occurs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @bogdan-iancu, I don't have the core file but if I remember correctly it was reproducible in the lab. Let me try to get one later today or tomorrow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @bogdan-iancu I was able to get the core file. I had to cherry-pick @a5b6d2e2b2f3e9eb72830410d3aaf1819326fe2a and put an abort to get it to dump.

#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007faa2e691535 in __GI_abort () at abort.c:79
#2  0x00007faa224fbcaf in mc_compact_cb (buf_p=0x7ffd888875c0, wh_list=0x7faa2af0f5e8, type=1, olen=0x7ffd888875bc) at compression.c:956
#3  0x00007faa224f9b1d in wrap_tm_func (t=0x7faa22a05e80, type=2, p=0x7ffd88887660) at compression.c:307
#4  0x00007faa224f9927 in wrap_tm_compact (t=0x7faa22a05e80, type=8192, p=0x7ffd88887660) at compression.c:278
#5  0x00007faa225d3f6f in run_trans_callbacks (type=8192, trans=0x7faa22a05e80, req=0x7faa2af0c230, rpl=0x0, code=0) at t_hooks.c:209
#6  0x00007faa225e4be7 in t_forward_nonack (t=0x7faa22a05e80, p_msg=0x7faa2af0c230, proxy=0x0, reset_bcounter=1, locked=1) at t_fwd.c:835
#7  0x00007faa225c6a4c in w_t_relay (p_msg=0x7faa2af0c230, flags=0x0, proxy=0x0) at tm.c:1236
#8  0x000055824cb8caf1 in do_action (a=0x7faa2a694800, msg=0x7faa2af0c230) at action.c:974
#9  0x000055824cb89377 in run_action_list (a=0x7faa2a6912f0, msg=0x7faa2af0c230) at action.c:158
#10 0x000055824cb89231 in run_actions (a=0x7faa2a6912f0, msg=0x7faa2af0c230) at action.c:123
#11 0x000055824cb896c0 in run_top_route (a=0x7faa2a6912f0, msg=0x7faa2af0c230) at action.c:241
#12 0x000055824cb98ddb in receive_msg (
    buf=0x55824cebfe00 <buf> "INVITE sip:[email protected]:5060 SIP/2.0\r\nVia: SIP/2.0/UDP 192.168.20.190:5060;branch=teGidaYiiVzxlowwCFqaPPYbLJCWghYxusYp\r\nFrom: <sip:[email protected]:5060>;tag=RLCwhxURwvjF\r\nTo: <sip:2000@192."..., len=738, rcv_info=0x7ffd88887b90, existing_context=0x0, msg_flags=0) at receive.c:213
#13 0x000055824cce025f in udp_read_req (si=0x7faa2a68eeb8, bytes_read=0x7ffd88887c58) at net/proto_udp/proto_udp.c:186
#14 0x000055824ccc2185 in handle_io (fm=0x7faa2a6ba590, idx=0, event_type=1) at net/net_udp.c:278
#15 0x000055824ccc0909 in io_wait_loop_epoll (h=0x55824cea9fc0 <_worker_io>, t=1, repeat=0) at net/../io_wait_loop.h:305
#16 0x000055824ccc33fb in udp_start_processes (chd_rank=0x55824cea9e7c <chd_rank>, startup_done=0x0) at net/net_udp.c:503
#17 0x000055824cbf9670 in main_loop () at main.c:802
#18 0x000055824cbfcc9d in main (argc=15, argv=0x7ffd88887f38) at main.c:1491

@liviuchircu
Copy link
Member

@john08burke it's been a while since I self-requested the review here, but I finally managed to find some time to work on this issue. So far, I managed to both reproduce both your LM_BUG log and the corruption/crash itself, while also catching two other mc_compact() bugs (fixes are pushed, you can check them out).

So now it's just a matter of understanding the crash and if your fix is optimal. Will come back to you soon with an update!

@john08burke
Copy link
Contributor Author

Hey @liviuchircu, glad you were able to reproduce! It's been a while, but I do remember feeling like this was a bit of a band-aid solution 😄 but wasn't able to find something to keep the short-circuit logic in play.

Feel free to close this one if you have a better patch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CRASH] Compression module memory corruption
3 participants