command: avoid crashing the player with mmap

mmap in inherently crashy due to the file changing underneath
and requires esoteric SIGBUS handling [0] to use properly.

to reproduce, simply use "overlay-add" from a lua script with
invalid stride set and observe mpv crashing with SIGBUS.

here we're just copying the file content (introduced in
549a9ea) and munmaping it immediately so there's no real need
to be using mmap to begin with. avoid the issue entirely by just
reading the file directly with fseek+fread with optimization for
the common stride case.

one notable difference is that since dup-ed fds share same
offset, the source "fd" that's coming from client will now have
it's offset modified. `pread` could avoid this issue but it's
not supported on windows.

[0]: https://www.sublimetext.com/blog/articles/use-mmap-with-care
This commit is contained in:
NRK
2026-03-12 15:58:53 +01:00
committed by sfan5
parent 1aee4637f2
commit 3cd66d2fd7
2 changed files with 53 additions and 30 deletions
+16 -11
View File
@@ -932,8 +932,9 @@ OSD Commands
``file`` specifies the file the raw image data is read from. It can be
either a numeric UNIX file descriptor prefixed with ``@`` (e.g. ``@4``),
or a filename. The file will be mapped into memory with ``mmap()``,
copied, and unmapped before the command returns (changed in mpv 0.18.1).
or a filename. The file will be read into memory before the command returns.
Note that since mpv 0.42.0, if a file descriptor is specified, the file
offset of the descriptor will be modified.
It is also possible to pass a raw memory address for use as bitmap memory
by passing a memory address as integer prefixed with an ``&`` character.
@@ -942,10 +943,6 @@ OSD Commands
memory address (since mpv 0.8.0, ignored before).
``offset`` is the byte offset of the first pixel in the source file.
(The current implementation always mmap's the whole file from position 0 to
the end of the image, so large offsets should be avoided. Before mpv 0.8.0,
the offset was actually passed directly to ``mmap``, but it was changed to
make using it easier.)
``fmt`` is a string identifying the image format. Currently, only ``bgra``
is defined. This format has 4 bytes per pixels, with 8 bits per component.
@@ -973,12 +970,20 @@ OSD Commands
.. note::
Before mpv 0.42.0, the file was mapped into memory with ``mmap()``,
copied, and unmapped before the command returns. Changing the file
before the command returns is highly discouraged as this could lead to
crashes on versions before 0.42.0.
Before mpv 0.18.1, you had to do manual "double buffering" when updating
an overlay by replacing it with a different memory buffer. Since mpv
0.18.1, the memory is simply copied and doesn't reference any of the
memory indicated by the command's arguments after the command returns.
If you want to use this command before mpv 0.18.1, reads the old docs
to see how to handle this correctly.
an overlay by replacing it with a different memory buffer. If you want
to use this command before mpv 0.18.1, read the old docs to see how to
handle this correctly.
Larger offsets should be avoided as mpv versions before 0.42.0 used to
mmap the whole file from position 0 to the end of the image. Before mpv
0.8.0, the offset was actually passed directly to ``mmap``. Since mpv
0.42.0, no mmap happens, so offset is handled by seeking on the file.
``overlay-remove <id>``
Remove an overlay added with ``overlay-add`` and the same ID. Does nothing
+37 -19
View File
@@ -5152,6 +5152,28 @@ static void replace_overlay(struct MPContext *mpctx, int id, struct overlay *new
recreate_overlays(mpctx);
}
static bool
fread_pic(FILE *fp, mp_image_t *dst, size_t bytesPerLine, size_t h, size_t stride, size_t offset)
{
size_t bytesRead = 0, expectedBytes = h * bytesPerLine;
size_t src_off = offset, dst_off = 0;
if (stride == dst->stride[0]) {
fseek(fp, src_off, SEEK_SET);
bytesRead = fread(dst->planes[0], 1, expectedBytes, fp);
} else {
for (int i = 0; i < h; i++) {
fseek(fp, src_off, SEEK_SET);
size_t r = fread(dst->planes[0] + dst_off, 1, bytesPerLine, fp);
if (r != bytesPerLine)
break;
bytesRead += bytesPerLine;
src_off += stride;
dst_off += dst->stride[0];
}
}
return bytesRead == expectedBytes;
}
static void cmd_overlay_add(void *pcmd)
{
struct mp_cmd_ctx *cmd = pcmd;
@@ -5189,14 +5211,12 @@ static void cmd_overlay_add(void *pcmd)
if (!overlay.source)
goto error;
int fd = -1;
bool close_fd = true;
void *p = NULL;
if (file[0] == '@') {
char *end;
fd = strtol(&file[1], &end, 10);
if (!file[1] || end[0])
fd = -1;
close_fd = false;
int srcfd = strtol(&file[1], &end, 10);
if (file[1] && end[0] == '\0' && srcfd >= 0)
fd = mp_dup_cloexec(srcfd); // fdopen will "own" the fd
} else if (file[0] == '&') {
char *end;
unsigned long long addr = strtoull(&file[1], &end, 0);
@@ -5206,24 +5226,22 @@ static void cmd_overlay_add(void *pcmd)
} else {
fd = open(file, O_RDONLY | O_BINARY | O_CLOEXEC);
}
size_t map_size = 0;
if (fd >= 0) {
map_size = offset + h * stride;
void *m = mmap(NULL, map_size, PROT_READ, MAP_SHARED, fd, 0);
if (close_fd)
close(fd);
if (m && m != MAP_FAILED)
p = m;
FILE *fp = (fd >= 0) ? fdopen(fd, "rb") : NULL;
int open_or_read_err = 1, bytesPerLine = w * 4;
if (p) {
memcpy_pic(overlay.source->planes[0], (char *)p + offset, bytesPerLine,
h, overlay.source->stride[0], stride);
open_or_read_err = 0;
} else if (fp) {
open_or_read_err = !fread_pic(fp, overlay.source, bytesPerLine, h, stride, offset);
fclose(fp);
}
if (!p) {
MP_ERR(mpctx, "overlay-add: could not open or map '%s'\n", file);
if (open_or_read_err) {
MP_ERR(mpctx, "overlay-add: could not open or read '%s'\n", file);
talloc_free(overlay.source);
goto error;
}
memcpy_pic(overlay.source->planes[0], (char *)p + offset, w * 4, h,
overlay.source->stride[0], stride);
if (map_size)
munmap(p, map_size);
replace_overlay(mpctx, id, &overlay);
return;