fte-team / fteqw

This is the official GitHub mirror for the FTEQW project.
https://www.fteqw.org/
159 stars 48 forks source link

segfault at end of replay on OpenBSD #129

Closed Xylemon closed 1 year ago

Xylemon commented 1 year ago

https://sourceforge.net/p/fteqw/tickets/81/

namtsui wrote on 2020-04-23:

Watching multiview demos (.mvd) is fine until end, when it consistently causes a segfault. This is due to cls.demoinfile potentially being null.

bind RIGHTARROW demojump +20 clgameclock 1* Watch until end of demo and it will segfault.

Thread 1 received signal SIGSEGV, Segmentation fault.
0x000009314bc47915 in readdemobytes (readpos=0x7f7ffffbccf4, data=0x7f7ffffbcd1c, len=4) at client/cl_demo.c:305
305         i = VFS_READ(cls.demoinfile, demobuffer+demooffset+demobuffersize, trybytes);
(gdb) bt
#0  0x000009314bc47915 in readdemobytes (readpos=0x7f7ffffbccf4, data=0x7f7ffffbcd1c, len=4) at client/cl_demo.c:305
#1  0x000009314bc491f0 in CL_GetDemoMessage () at client/cl_demo.c:694
#2  0x000009314bc8b5a9 in CL_ReadPackets () at client/cl_main.c:3986
#3  0x000009314bc91ca2 in Host_Frame (time=0.014754999999922802) at client/cl_main.c:6107
#4  0x000009314baaa88f in main (c=3, v=0x7f7ffffbd3e8) at client/sys_linux.c:1202
(gdb) print cls.demoinfile
$2 = (vfsfile_t *) 0x0

This patch seems to resolve the issue where I add checks for cls.demoinfile being null to immediately return.

$OpenBSD$

fixes segfault at end of demos where cls.demoinfile can be a null pointer

Index: engine/client/cl_demo.c
--- engine/client/cl_demo.c.orig
+++ engine/client/cl_demo.c
@@ -301,9 +301,13 @@ int readdemobytes(int *readpos, void *data, int len)
    }

    trybytes = sizeof(demobuffer)-demooffset-demobuffersize;
-   if (trybytes > 4096)
+   if (trybytes > 4096) {
+       if (!cls.demoinfile) {
+           CL_StopPlayback ();
+           return 0;
+       }
        i = VFS_READ(cls.demoinfile, demobuffer+demooffset+demobuffersize, trybytes);
-   else
+   } else
        i = 0;
    if (i > 0)
    {
@@ -316,6 +320,11 @@ int readdemobytes(int *readpos, void *data, int len)

    if (*readpos+len > demobuffersize)
    {
+       if (!cls.demoinfile) {
+           CL_StopPlayback ();
+           return 0;
+       }
+
        if (i < 0 || (i == 0 && len && cls.demoinfile->seekstyle < SS_SLOW))
        {   //0 means no data available yet, don't error on that (unless we can seek, in which case its not a stream and we won't get any more data later on).
            endofdemo = true;
Xylemon commented 1 year ago

I know we are in the middle of moving to Github, but I just wanted to take a moment to say thank you for testing and patching issues on OpenBSD :)

namtsui commented 1 year ago

you can close this. it works testing against latest git version on openbsd.