> From: fengchengwen [mailto:fengcheng...@huawei.com] > Sent: Wednesday, 15 November 2023 04.03 > > On 2023/11/15 1:49, Tyler Retzlaff wrote: > > On Tue, Nov 14, 2023 at 09:34:33AM -0800, Tyler Retzlaff wrote: > >> On Tue, Nov 14, 2023 at 09:32:48AM -0800, Tyler Retzlaff wrote: > >>> On Tue, Nov 14, 2023 at 08:50:17PM +0800, Jie Hai wrote: > >>>> On 2023/11/14 1:09, Tyler Retzlaff wrote: > >>>>> On Mon, Nov 13, 2023 at 06:45:29PM +0800, Jie Hai wrote: > >>>>>> Multiple threads calling the same function may cause condition > >>>>>> race issues, which often leads to abnormal behavior and can > cause > >>>>>> more serious vulnerabilities such as abnormal termination, > denial > >>>>>> of service, and compromised data integrity. > >>>>>> > >>>>>> The strtok() is non-reentrant, it is better to replace it with a > >>>>>> reentrant function. > >>>>> > >>>>> could you please use strtok_s instead of strtok_r the former is > part of > >>>>> the C11 standard the latter is not. > >>>>> > >>>>> thanks! > >>>>> > >>>> Hi, Tyler Retzlaff > >>>> > >>>> Thanks for your comment. > >>>> > >>>> For the use of strtok_s, I have consulted some documents, see > >>>> https://en.cppreference.com/w/c/string/byte/strtok > >>>> It says > >>>> "As with all bounds-checked functions, strtok_s only guaranteed to > be > >>>> available if __STDC_LIB_EXT1__ is defined by the implementation > and if > >>>> the user defines __STDC_WANT_LIB_EXT1__ to the integer constant 1 > before > >>>> including <string.h>. > >>>> " > >>>> > >>>> I use strtok_s with "#ifdef __STDC_LIB_EXT1__ ... #endif" around > and > >>>> compiled > >>>> locally and found that __STDC_LIB_EXT1__ was not defined, so the > related > >>>> code was not called. I'm afraid there's a problem with this > >>>> modification. > >>>> > >>>> Am I using strtok_s wrong? > >>> > >>> no i overlooked that it is optional my fault sorry. > >>> > >>> since there is no portable strtok_r i'm going to have to agree with > others > >>> that only places where you actually need reentrant strtok be > converted to > >>> strtok_r. > >>> > >>> for windows i think we'll either need to introduce an abstracted > strtok_r > >>> name for portability or something in the rte_ namespace (dependent > on > >>> what other revieweres would prefer) > >> > >> just as a follow up here maybe it would be optimal if we could use > >> strtok_s *if* we test that the toolchain makes it available and if > not > >> provide a mapping of strtok_s -> strtok_r. > > > > just a final follow up, i can see that we already have a rte_strerror > > here to do the replace with reentrant dance. it is probably good to > > follow the already established pattern for this and have a > rte_strtok. > > +1 for have rte_strtok which could cover different platform.
+1 to rte_strtok doing the reentrant dance for us. If we had such an rte_strtok(), we could also generally disallow the use of strtok(). > > > > > what do others think? > >
