#333 LAME Memory Overwrite

[paul_sanders]

Dear LAME developers,

I came across a bug in the Windows LAME encoder DLL which I thought I should report. It is a memory overwrite bug and potentially affects users of LAME in its various forms on all platforms in various subtle and unpleasant ways, hence this email.

Encoding this file:

http://www.alpinesoft.co.uk/private/sonic_bodyshop~Vinyl~side1.wav

With these encoding parameters:
VBR 0
VBR new
44.1kHz
BE_JOINT_STEREO
bit reservoir enabled
copyright bit off
CRC bit off
original bit off
private bit off (no jokes please)
bitrate_min = bitrate_max = 0
mpeg version = MPEG1

Causes LAME to write past the end of the output buffer, assuming that one allocates it as the value returned by BEINITSTREAM in dwBufferSize. Specifically, it wrote 9367 bytes to an 8604 byte buffer at some point in the encoding process. Only 8604 bytes (or less) were actually returned, but while encoding the input buffer LAME wrote to memory beyond that.

I am using lamenc.dll 3.98.2, filesize 484352 bytes (not sure of the file date). I can't give you a simple exe to work with (unless you want a copy of VinylStudio) because I use it in an 'end-user' application (of which I am the developer). Maybe you can reproduce it with the command line LAME encoder though, I haven't tried.

To trap this bug at the point of transgression (under Windows), obtain a copy of the Debugging Tools for Windows, track down gflags.exe therein and run:

gflags -p /decommit /enable LAME.exe

(or whatever EXE you are using to try to reproduce this bug).

Then do your encoding run and you should trap into the debugger (in what looks like some kind of memcpy function) as soon as it tries to write beyond the end of the buffer. No doubt similiar tools exist on other platforms (do they?).

Please let me know if there's anything else I can do to help reproduce this. The workaround is simple of course: the caller can allocate a larger buffer than recommended by BEINITSTREAM. x 2 in my case, hope it's big enough :)

There's a memory leak too. I havent looked into it in detail, but the sequence BEINITSTREAM, BEENCODECHUNK..., BEDEINITSTREAM, BECLOSESTREAM definitely leaves some memory allocated. I believe my own code tidies up as it should. I solve this by unloading and reloading the DLL periodically.

Great encoder by the way, glitches or no, love it to bits. Why is there no 'group' for bugs in this webform?

Regards,

Paul Sanders
AlpineSoft
http://www.alpinesoft.co.uk

[Robert Hegemann]

Paul,

the old Blade replacement DLL has some oddities:
- it estimates some buffer size (is the formula correct?), but doesn't allocate it itself;
- at one point it passes a buffer size of LAME_MAXMP3BUFFER into LAME library, even though the above mentioned formula estimates the buffer size smaller than LAME_MAXMP3BUFFER and DLL doesn't know how many bytes you really allocated;
- at some places it passes 0 as buffer size into LAME library, with the meaning of "don't care about it, it will be large enough anyway";

So, if you allocate LAME_MAXMP3BUFFER bytes as a temporary solution, that should work for now.

Re Memory-Leak. Maybe lame_close doesn't get called. In case the mp3 your are generating should have some Info/Xing/VBR-Tag (LHV1.bWriteVBRHeader=TRUE), you need to call beWriteInfoTag just before beCloseStream. If you don't do that, LAME will not free any allocated memmory.

Ciao Robert

[Nobody/Anonymous]

Thank you Robert, that is very helpful. There are indeed times when I set LHV1.bWriteVBRHeader but don't subsequently write the header to file. Some quick tests show that beWriteVBRHeader (which is what I use when I do want a header written out) also seems to free the memory. I can't find any documentation on beWriteInforTag anywhere so I don't use it.

lame_enc.dll seems to be a bit of a poor relation. Maybe the command line parser could be moved into the DLL so that we could pass in the same options as used by LAME.exe. Just a thought.

The buffer size calculation you refer to seems to be correct in terms of the maximum frame size that beEncodeChunk will return, but, as we now know, that's not the whole story. Thanks again for the information.

Regards - Paul Sanders.