> Ok, I can submit a patch which uses C++11 thread_local when > PODOFO_MULTI_THREAD is defined. The recursion guard definition will look like > this: > > #if defined(PODOFO_MULTI_THREAD) > static int thread_local s_nRecursionDepth; // PoDoFo with threading support > requires C++11 compiler with thread_local > #else > static int s_nRecursionDepth; // PoDoFo is single threaded > #endif > > Does that work for everyone?
Looks good to me, and the comment is hopefully explanation enough if anyone runs into a compile time error. Please do include a doc patch stating the requirement. Can we get a macro that creates this thread-local integer and the recursion guard object all in one go, with the connotation that the recursion guard is meant to usually be applied to each affected function entry point separately? (Unless that is not what it is meant to do. I think we could just as well make an argument for a single recursion depth counter per thread, which then probably should become a static member of the recursion guard class.) Cheers, Christopher The MathWorks GmbH | Friedlandstr.18 | 52064 Aachen | District Court Aachen | HRB 8082 | Managing Directors: Bertrand Dissler, Steven D. Barbo, Jeanne O’Keefe From: Mark Rogers <mark.rog...@powermapper.com> Sent: Thursday, November 25, 2021 16:33 To: Christopher Creutzig <ccreu...@mathworks.com>; Michal Sudolsky <sudols...@gmail.com> Cc: podofo-users@lists.sourceforge.net Subject: Re: [Podofo-users] PoDoFo and recursive stack consumption CVEs >>> I like this idea. As PODOFO_MULTI_THREAD will enable C++11 >>Is there a chance we might get there? Who would be able to make that decision? >consider the decision being made. Again, from my point of view. In >other words, feel free to provide a patch with the suggested changes. Ok, I can submit a patch which uses C++11 thread_local when PODOFO_MULTI_THREAD is defined. The recursion guard definition will look like this: #if defined(PODOFO_MULTI_THREAD) static int thread_local s_nRecursionDepth; // PoDoFo with threading support requires C++11 compiler with thread_local #else static int s_nRecursionDepth; // PoDoFo is single threaded #endif Does that work for everyone? > Not when the user of podofo already used some 64 KB before calling podofo. To > me it seems more reasonable to use a more conservative value which would not > consume more than some half (or tenth?) of the available stack in the worst > case. I’ll also reduce the 500 max recursion depth as suggested (probably to 256) And I’ll also include the new parser unit tests which test for deep recursion and reference loops We’re also testing a patch for CVE-2018-20797. This is caused by an invalid negative value for one of the FlateDecode compression parameters which results in a call to podofo_calloc( -14 ) == podofo_calloc( 0xfffffffffffffff2 ) Best Regards Mark Mark Rogers - mark.rog...@powermapper.com<mailto:mark.rog...@powermapper.com> PowerMapper Software Ltd - www.powermapper.com<http://www.powermapper.com> Registered in Scotland No 362274 Quartermile 2 Edinburgh EH3 9GL From: Christopher Creutzig <ccreu...@mathworks.com<mailto:ccreu...@mathworks.com>> Date: Thursday, 25 November 2021 at 07:16 To: Michal Sudolsky <sudols...@gmail.com<mailto:sudols...@gmail.com>> Cc: "PowerMapper.com" <mark.rog...@powermapper.com<mailto:mark.rog...@powermapper.com>>, "podofo-users@lists.sourceforge.net<mailto:podofo-users@lists.sourceforge.net>" <podofo-users@lists.sourceforge.net<mailto:podofo-users@lists.sourceforge.net>> Subject: RE: [Podofo-users] PoDoFo and recursive stack consumption CVEs >> If we want to avoid UB in the multithreaded world, I’m afraid we will have >> to make a C++11 compiler a requirement, as C++03 never acknowledged the >> existence of threads. (That is not limited to this place, a lot of methods >> like PdfEncodingFactory::GlobalPdfRomanEncodingInstance are not currently >> threadsafe in C++03, as discussed earlier.) > That is not thread-safe even in C++11. True, but C++11 or later would give us the tools to make it thread-safe. > Except that some things are not so available as threads like for example > thread_local and atomic operations. thread_local equivalents are available for g++, clang, and MSVC. That covers the compilers listed in https://github.com/NickNaso/PoDoFo/blob/master/README.md#installation_with_cmake<https://github.com/NickNaso/PoDoFo/blob/master/README.md#installation_with_cmake>. See my proposed PODOFO_THREAD_LOCAL in https://sourceforge.net/p/podofo/mailman/message/37389082/<https://sourceforge.net/p/podofo/mailman/message/37389082/>. > I like this idea. As PODOFO_MULTI_THREAD will enable C++11 Is there a chance we might get there? Who would be able to make that decision? Cheers, Christopher The MathWorks GmbH | Friedlandstr.18 | 52064 Aachen | District Court Aachen | HRB 8082 | Managing Directors: Bertrand Dissler, Steven D. Barbo, Jeanne O’Keefe
_______________________________________________ Podofo-users mailing list Podofo-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/podofo-users
