2017.07.04 12:30 "Re: [Tiff] A bug in libtiff error/warning handling", by Paavo Helde

On 4.07.2017 14:30, Even Rouault wrote:

On mardi 4 juillet 2017 14:04:35 CEST Paavo Helde wrote:

I would like to report what I think is a bug in libtiff error and warning handling. There are two error handlers which can be installed (via TIFFSetErrorHandler and TIFFSetErrorHandlerExt) and which are called with a va_list. However, if both handlers are installed they will both iterate through the same va_list without reinitialization which is not allowed (seems to crash randomly with gcc on Linux, for example). I believe it should be the task of libtiff to reinitialize va_list between the calls. Ditto for warnings.

A patch file is attached, hopefully in a usable format.

That's a good point, but I wonder why you would have both error handlers installed? That isn't really expected.

The standard handler is initialized to unixErrorHandler by default and there is no indication I should set it to NULL when installing the extended error handler. Also, what I'm developing is a library which works in the same process with several other third-party libraries making use of the same libtiff library instance, some of which install their own handlers (not too many of them luckily, otherwise this would be a much more severe problem). When I reviewed common libraries which we share process with I discovered they are using TIFFSetErrorHandler(), that's the main reason why I used TIFFSetErrorHandlerExt() myself, to reduce conflicts with other libraries (however, I do not attempt to dereference the passed-in tif->tif_clientdata pointer because this may actually point to foreign data (e.g. libTIFF was called in another thread by another third-party library and has actually nothing to do with our library)).

A proper fix would be to deprecate the global handler variables in libtiff (which are not thread-safe anyway and will conflict between different libraries anyway) and store the handler function pointer in the TIFF structure instead, next to tif_clientdata. This would make the handler specific to a single client and then I would indeed agree there would be no need to call more than one handler at any time. I think it has been agreed earlier that this is something what is needed, but to get this fixed is obviously much more work.

Cheers

Paavo