Closed Polochon-street closed 1 year ago
The error probably stems from https://github.com/MusicPlayerDaemon/MPD/blob/master/src/system/Error.hxx#L94-L95 , and the fact that
snprintf
returns the number of bytes which would have been written to the final string if enough space had been available (https://linux.die.net/man/3/snprintf), and not the number of bytes actually written.
Oh no, that's indeed a stupid mistake!
it's definitely a footgun - now that I think about it, there are probably other places where that happens, I should look into it :)
There are few places where the return value is really used, and it looks like those are used correctly (to allocate a buffer). MPD is being migrated away from those poor C APIs; the transition to libfmt is going on in the master branch, though this mistake was stil present over there.
oh, alright. Awesome, and thanks for the quick fix!
FWIW this also hits linux, particularly on AudioCD playback. 0.23.10 had very frequent IO crashes (0.23.9 was mostly fine): 0.23.11 apparently fixes it (not windows-only fix). Thanks!
@MaxKellermann While it's better, but not fully solved yet: have some remaining IO struggles crashing mpd thread when reading Audio CD. Thoughts? What/how can I provide meaningful logs?
What/how can I provide meaningful logs?
Thanks for the tip.
While I'm trying to rebuild (on Alpine/musl) with debub symbols for use with gdb
, I noticed a bunch of compile warnings for src/decoder/plugins/libdecoder_plugins.a.p/FfmpegIo.cxx.o
in case this may be related:
https://pastebin.com/3Sz02H8b
I don't see any compiler warnings in your paste
If those are no concerning messages, then fine.
For some reason, under gdb
can't get pass:
Failed to decode cdda:///1; Unable find or access a CD-ROM drive with an audio CD in it.
https://pastebin.com/76qWTZ3d
While launched regularly it does access CD and then hangs. (IO stuff I'd like to debug)
in both cases mpd
process seems launched as mpd
user as expected, which is part of cdrom
and audio
groups... :eyes:
I see there are many fmt
buffer-related patches in master
since 0.23.11, some related to cdio
, so thought I would try with master
.
However, build stumbles upon:
ninja: job failed: /usr/bin/sphinx-build -q -b html -d doc/doctrees /builds/macmpi/aports/community/mpd/src/MPD-8b1ff3f0055295b50f29a7af09809772484ef645/doc doc/html
WARNING: sphinx_rtd_theme (< 0.3.0) found. It will not be available since Sphinx-6.0
Theme error:
no theme named 'sphinx_rtd_theme' found (missing theme.conf?)
/builds/macmpi/aports/community/mpd/src/MPD-8b1ff3f0055295b50f29a7af09809772484ef645/doc/user.rst:720: ERROR: Unexpected indentation.
/builds/macmpi/aports/community/mpd/src/MPD-8b1ff3f0055295b50f29a7af09809772484ef645/doc/user.rst:723: ERROR: Unexpected indentation.
ninja: subcommand failed
Thoughts?
Can't get much out of gdb
: it's not a crash here, but IO that ramps-up up at 90% and process that seems stuck (D flag in htop).
gdb
would not give me a promt to do any backtrace.
Same with current master (reverting https://github.com/MusicPlayerDaemon/MPD/commit/6913148d994f6595e9c5ee67fca3f6fd6b0e46c5 to build)
Bug report
Describe the bug
When sending 1. large input and 2. that would trigger the logging of an error on MPD on windows, MPD just segfaults. For instance, sending
listplaylist <400 times 'A'>
swiftly makes MPD segfaults.The error probably stems from https://github.com/MusicPlayerDaemon/MPD/blob/master/src/system/Error.hxx#L94-L95 , and the fact that
snprintf
returns the number of bytes which would have been written to the final string if enough space had been available (https://linux.die.net/man/3/snprintf), and not the number of bytes actually written. I think changing it by a min(return_value, 512) would work and I'd gladly submit a PR if that's the way to go :)It also means that it's possible to write
0x3a20
(which corresponds to the:
manually written) about anywhere on the stack, which leads to crashes when it's written to, for instance, rip, but could probably be used for more malicious purposes.Expected Behavior
MPD does not crash when sent arbitrary big input.
Actual Behavior
MPD crashes when sent big payloads.
Version
Configuration
Log
The backtrace is misleading, since we're dealing with stack frame corruptions here, but added a few excerpts, depending on the length of the payload:
or