Opened 4 years ago
Closed 4 years ago
#850 closed defect (fixed)
Off by one bug in src/lib/mr_str.c
Reported by: | KONG | Owned by: | Bruno Cornec |
---|---|---|---|
Priority: | high | Milestone: | 3.3.0 |
Component: | mondo | Version: | 3.2.2 |
Severity: | major | Keywords: | |
Cc: |
Description
I found a bug in src/lib/mr_str.c. The following line:
q = outstr + strlen(outstr) -1;
will cause a SIGABRT on latest rhel 7. See parts of the log:
*** Error in `/usr/sbin/mondoarchive': corrupted size vs. prev_size: 0x000000000065b5d0 *** ======= Backtrace: ========= /lib64/libc.so.6(+0x7f7c4)[0x7ffff765b7c4] /lib64/libc.so.6(+0x82fd4)[0x7ffff765efd4] /lib64/libc.so.6(+0x840c2)[0x7ffff76600c2] /lib64/libc.so.6(realloc+0x1d2)[0x7ffff7662162] /lib64/libc.so.6(getdelim+0x10b)[0x7ffff764b91b] /usr/sbin/mondoarchive[0x4380bf] /usr/sbin/mondoarchive[0x41e765] /usr/sbin/mondoarchive[0x4291c8] /usr/sbin/mondoarchive[0x402e90] /lib64/libc.so.6(__libc_start_main+0xf5)[0x7ffff75fe505] /usr/sbin/mondoarchive[0x4039b3] == ....
Running it through valgrind, displays:
I think you have a EFI/UEFI partition. ==2587== Invalid read of size 1 ==2587== at 0x437DAA: mr_strip_spaces (mr_str.c:185) ==2587== by 0x41E6FC: run_program_and_log_output (libmondo-fork.c:317) ==2587== by 0x42B079: some_basic_system_sanity_checks (libmondo-tools.c:1016) ==2587== by 0x42B4A3: pre_param_configuration (libmondo-tools.c:684) ==2587== by 0x402E46: main (mondoarchive.c:328) ==2587== Address 0x5ed05bf is 1 bytes before a block of size 1 alloc'd ==2587== at 0x4C29EA3: malloc (vg_replace_malloc.c:309) ==2587== by 0x537FF2F: __vasprintf_chk (vasprintf_chk.c:80) ==2587== by 0x437FFA: UnknownInlinedFun (stdio2.h:210) ==2587== by 0x437FFA: mr_asprintf_int (mr_mem.c:62) ==2587== by 0x437D98: mr_strip_spaces (mr_str.c:181) ==2587== by 0x41E6FC: run_program_and_log_output (libmondo-fork.c:317) ==2587== by 0x42B079: some_basic_system_sanity_checks (libmondo-tools.c:1016) ==2587== by 0x42B4A3: pre_param_configuration (libmondo-tools.c:684) ==2587== by 0x402E46: main (mondoarchive.c:328) ==2587==
The following change fixes it:
q = outstr + strlen(outstr);
After that archive creation successfully completes.
Change History (3)
comment:1 by , 4 years ago
Priority: | normal → high |
---|---|
Severity: | normal → major |
Status: | new → assigned |
comment:2 by , 4 years ago
Just tested the fix, but I think you made an error. With your fix I get a message after Checking sanity of your linux distribution: "You must have at least 32MB of RAM to use Mondo".
But this server has 128GB of RAM
Your fix checks for instr != '\0' :
if (*instr != '\0') { mr_asprintf(outstr, ""); return(outstr); }
I think what you wanted to write is intstr == '\0':
if (*instr == '\0') { mr_asprintf(outstr, ""); return(outstr); }
With this archive creation works again as expected.
comment:3 by , 4 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Yes you're right ! Made the test wrong :-( Fixed then better with [3724]
Thanks for your report.
Should be fixed by [3723].
Your proposed patch is IMO incorrect, except for empty strings, so it should crash elsewhere.
Please try applying mine to see if it is also fixing your case, without creating other issues.