[boutique]

Funnily enough, on Half-Life 1 engine-based games (i.e. the engine that came before HL2 - on which Team Fortress 2 runs; such as Counter-Strike 1.6), a different allocator problem exists -- glibc's malloc() just decides to fail miserably[0] on some setups.

[0] https://github.com/ValveSoftware/halflife/issues/3158

[kimixa]

that's exactly the sort of error you get if something has written just out of bounds on a malloc'd chunk - it clobbers the allocator's internal state, which appears to be what that assert() is checking.

It's probably an allocation before the failing one that is being misued - so the backtrace pointing to openal doesn't necessarily mean it's openal's fault.

Running with valgrind or another heap memory checking tool will probably be helpful to track down that particular linked bug.

EDIT:

It looks like that there's at least one out-of-bounds write when starting up half life (On arch linux, so maybe slightly different library versions and not loaded the counterstrike mod).

It looks like a valve bug - writing 2 bytes at index [30] of a malloc'd size of 31 goes one byte over, and it looks from the backtrace it's all valve's code and not deep in some library that might have been loaded in. Writing 2 bytes to a string is a bit odd, perhaps it's trying to null-terminate but somehow uses a wstring null? Or some attempt at SIMD that isn't correctly bound?

It doesn't seem to crash for me though, it might just be luck that nothing important is put 1 byte over, and it feels a bit unlikely something would be due to allocation and type alignment requirements, but it's perfectly valid for the malloc implementation to keep something important in that byte.

Or perhaps there's some other dynamics that change this - it looks like it's doing stuff with paths, so may change size (of the allocation or even the amount written) based on where the steam app is installed - stuff like your user name length changing that may be the difference between a crash. Or even another issue somewhere else I didn't see, or valgrind didn't catch.

Just goes to show how many games ship for years with "big" bugs :P

For reference:

  ==27467== Invalid write of size 2                                                                                                                                                                                                            
  ==27467==    at 0x406526A: GetSteamContentPath() 
  (pathmatch.cpp:523)
  ==27467==    by 0x4065927: pathmatch(char const*, char\*, 
  bool, char*, unsigned int) [clone .part.1] (pathmatch.cpp:594)
  ==27467==    by 0x4066849: pathmatch (pathmatch.cpp:541)
  ==27467==    by 0x4066849: CWrap (pathmatch.cpp:685)
  ==27467==    by 0x4066849: __wrap___xstat (pathmatch.cpp:907)
  ==27467==    by 0x406294A: stat (stat.h:455)
  ==27467==    by 0x406294A: CFileSystem_Stdio::FS_stat(char const*, stat*) (FileSystem_Stdio.cpp:225)
  ==27467==    by 0x4060819: CBaseFileSystem::AddPackFiles(char const*) (BaseFileSystem.cpp:1325)
  ==27467==    by 0x4060AA4: CBaseFileSystem::AddSearchPathInternal(char const*, char const*, bool) (BaseFileSystem.cpp:254)
  ==27467==    by 0x4060B37: CBaseFileSystem::AddSearchPath(char const*, char const*) (BaseFileSystem.cpp:186)
  ==27467==    by 0x8049003: main (launcher.cpp:413)
  ==27467==  Address 0x45e5f4e is 30 bytes inside a block of size 31 alloc'd
  ==27467==    at 0x4041714: malloc (vg_replace_malloc.c:393)
  ==27467==    by 0x4357C4A: strdup (strdup.c:42)
  ==27467==    by 0x42F1A76: realpath_stk (canonicalize.c:410)
  ==27467==    by 0x42F1A76: realpath@@GLIBC_2.3 (canonicalize.c:432)
  ==27467==    by 0x406525B: GetSteamContentPath() (pathmatch.cpp:520)
  ==27467==    by 0x4065927: pathmatch(char const*, char\*, bool, char*, unsigned int) [clone .part.1] (pathmatch.cpp:594)
  ==27467==    by 0x4066849: pathmatch (pathmatch.cpp:541)
  ==27467==    by 0x4066849: CWrap (pathmatch.cpp:685)
  ==27467==    by 0x4066849: __wrap___xstat (pathmatch.cpp:907)
  ==27467==    by 0x406294A: stat (stat.h:455)
  ==27467==    by 0x406294A: CFileSystem_Stdio::FS_stat(char const*, stat*) (FileSystem_Stdio.cpp:225)
  ==27467==    by 0x4060819: CBaseFileSystem::AddPackFiles(char const*) (BaseFileSystem.cpp:1325)
  ==27467==    by 0x4060AA4: CBaseFileSystem::AddSearchPathInternal(char const*, char const*, bool) (BaseFileSystem.cpp:254)
  ==27467==    by 0x4060B37: CBaseFileSystem::AddSearchPath(char const*, char const*) (BaseFileSystem.cpp:186)
  ==27467==    by 0x8049003: main (launcher.cpp:413)

[boutique]

Thanks a lot for the guidance/tip, I've learned something new. And you're absolutely right about the cause of the mentioned crash -- I've updated the Github issue with a bit of new info I've gathered.

Regarding the function, here it is: https://github.com/dreamstalker/rehlds/blob/master/rehlds/fi...

Interestingly, strdup gets compiled into:

  89 04 24           mov   [esp+101Ch+name], pszContentPath ; s
  E8 82 DC 00 00     call  strlen
  66 C7 04 03 2F 00  mov   word ptr [pszContentPath+eax], 2Fh ; '/'

Which is basically:

  *(_WORD *)&pPath[strlen(pPath)] = '/';`

and would explain why Valgrind says it goes one byte over.

[kimixa]

Yeah, looks like the Q_strcat(pszContentPath, "/"); is invalid, as glibc has only allocated exactly enough to fit the path in the buffer returned by realpath().

The compiler seems to completely inline the strcat and write the '/' and null as a single 2-byte word write, the null then being out of bounds of the malloc'd chunk and likely causing the error as it overwrites something important.

Interestingly, the open group spec says that a null argument to realpath is "Implementation defined" [0]

And the linux (glibc) man pages say it allocates a buffer "Up to PATH_MAX" [1]

I guess "strlen(path)" is "Up to PATH_MAX", but the man page seems unclear - you could read that as implying the buffer is always allocated to PATH_MAX size, but that's not what seems to be happening, just effectively calling strdup() [2]. I have no idea how to feed back to the linux man pages, but might be worth clarifying there.

[0] https://pubs.opengroup.org/onlinepubs/009696799/functions/re...

[1] https://linux.die.net/man/3/realpath

[2] https://github.com/bminor/glibc/blob/0b9d2d4a76508fdcbd9f421...