October 07, 2021 03:48PM
>
> -----Original Message-----
> From: nginx-devel <nginx-devel-bounces@nginx.org> On Behalf Of Maxim Dounin
> Sent: Thursday, 7 October 2021 20:17
> To: nginx-devel@nginx.org
> Subject: Re: Sending a notification to the main nginx thread
>
>
> Hello!
>
> In no particular order:
>
> - Assuming nginx uses only POSIX functions is wrong, it does use
> platform-specific functions and various portable functions not
> specified by POSIX, such as
>
> - The above objdump results look incorrect: for example, nginx
> certainly uses readdir(), which is in the POSIX non-thread-safe
> list, but not in your list.
>
> - There are other libraries nginx uses, which makes the problem
> much worse.
>
> If you insist on using threads in your module - you are free to do so, you were warned and it's your choice.
>

Thanks Maxim, did some more research following your feedback, just for my understanding...
I'm sharing the results here in case someone will find it useful.

First of all, regarding readdir, you are right, of course.
The reason it was missing is that it shows as readdir64 in the import table,
while the list of non-thread-safe POSIX functions lists only readdir.

I ran a looser check - grepped the unsafe POSIX functions, but without assuming an exact match -

objdump -T /usr/local/nginx/sbin/nginx | grep -v ngx_ | grep -e asctime -e basename ... -e wctomb
0000000000000000 DF *UND* 0000000000000000 GLIBC_2.2.5 getenv
0000000000000000 DF *UND* 0000000000000000 GLIBC_2.2.5 localtime
0000000000000000 DF *UND* 0000000000000000 GLIBC_2.2.5 getpwnam
0000000000000000 DF *UND* 0000000000000000 GLIBC_2.2.5 getgrnam
0000000000000000 DF *UND* 0000000000000000 GLIBC_2.2.5 readdir64
0000000000000000 DF *UND* 0000000000000000 GLIBC_2.2.5 strerror
0000000000000000 DF *UND* 0000000000000000 GLIBC_2.2.5 dlerror

0000000000000000 DF *UND* 0000000000000000 GLIBC_2.2.5 srandom
0000000000000000 DF *UND* 0000000000000000 GLIBC_2.2.5 random
0000000000000000 DF *UND* 0000000000000000 GLIBC_2.2.5 localtime_r
0000000000000000 DF *UND* 0000000000000000 GLIBC_2.2.5 crypt_r
0000000000000000 DF *UND* 0000000000000000 GLIBC_2.2.5 gmtime_r

Other than readdir64, this added only a few false positives.

Regarding external dependencies, I looked at the basic deps used by nginx (didn't check more esoteric deps, such as libxslt etc.)
zlib & pcre claim to be thread-safe, openssl seems more problematic...
Didn't dig too deep, but seems that at least on older versions, you have to explicitly provide some callbacks to make it thread safe.

I then proceeded to pull a list of all POSIX functions -
(for i in {a..z}; do curl -s https://pubs.opengroup.org/onlinepubs/9699919799/idx/i$i.html | grep '()' | awk '-F(' '{print $1}' | awk '-F>' '{print $NF}'; done) > /tmp/posix-list

Compiled nginx with some basic settings -
auto/configure --with-file-aio --with-http_ssl_module --with-http_v2_module --with-stream --with-debug --with-threads --with-cc-opt="-O0" ; make install

And checked the imports while excluding POSIX, zlib, pcre & openssl -

(objdump -T /usr/local/nginx/sbin/nginx | grep UND | grep -v OPENSSL | grep -v pcre_ | grep -v deflate | awk '{print $NF}' ; cat /tmp/posix-list /tmp/posix-list) | sort | uniq -c | grep -w 1

1 accept4
1 __cmsg_nxthdr
1 crypt_r
1 epoll_create
1 epoll_ctl
1 epoll_wait
1 __errno_location
1 eventfd
1 ftruncate64
1 __fxstat64
1 __fxstatat64
1 getpagesize
1 getrlimit64
1 glob64
1 globfree64
1 __gmon_start__
1 initgroups
1 _ITM_deregisterTMCloneTable
1 _ITM_registerTMCloneTable
1 _Jv_RegisterClasses
1 __libc_start_main
1 __lxstat64
1 mmap64
1 open64
1 openat64
1 posix_fadvise64
1 prctl
1 pread64
1 pwrite64
1 pwritev64
1 readdir64
1 sched_setaffinity
1 sendfile64
1 setrlimit64
1 __stack_chk_fail
1 statfs64
1 syscall
1 usleep
1 __xstat64

Other than readdir64, which was already discussed, I don't spot any non-thread-safe functions, but maybe I'm missing something
(have to admit, I'm not familiar with some of the weird ones here...)

Bottom line, if we put aside openssl which is a bit unclear (I don't use it in my use case, so don't care much ;-)),
the problematic library functions used by nginx (at least on Linux...) seem to be - strerror, localtime, readdir.

Even though I agree with the general statement that thread use in nginx should be discouraged, sometimes it's the lesser evil...
IMHO, would be better to update nginx to use the thread-safe / _r variants on platforms that support them.
I don't see any downside to it, other than a couple more #if's in the code...
The world would have been better if such unsafe functions would have never been born :)

Thank you for your time!

Eran

> --
> Maxim Dounin
> https://eur02.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmdounin.ru%2F&amp;data=04%7C01%7C%7C60f75b5309df4989493108d989b64f04%7C0c503748de3f4e2597e26819d53a42b6%7C1%7C0%7C637692238383728263%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=2g%2FXvh1VsIgO%2F3jBOYQF%2Ba%2Fo3QfQFKUNcjZ5jCN%2Fke8%3D&amp;reserved=0
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> https://eur02.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmailman.nginx.org%2Fmailman%2Flistinfo%2Fnginx-devel&amp;data=04%7C01%7C%7C60f75b5309df4989493108d989b64f04%7C0c503748de3f4e2597e26819d53a42b6%7C1%7C0%7C637692238383728263%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=znw%2FdNVtzw6gd42ceMMeUgM20JbGvQClpksJYidQx8M%3D&amp;reserved=0
>
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

Sending a notification to the main nginx thread

erankor 594 October 05, 2021 10:06AM

Re: Sending a notification to the main nginx thread

Maxim Dounin 234 October 06, 2021 09:12AM

RE: Sending a notification to the main nginx thread

erankor 373 October 06, 2021 03:54PM

Re: Sending a notification to the main nginx thread

Maxim Dounin 284 October 07, 2021 09:08AM

RE: Sending a notification to the main nginx thread

erankor 353 October 07, 2021 11:18AM

Re: Sending a notification to the main nginx thread

Maxim Dounin 247 October 07, 2021 01:18PM

RE: Sending a notification to the main nginx thread

erankor 347 October 07, 2021 03:48PM

Re: Sending a notification to the main nginx thread

dnj0496 310 October 07, 2021 01:28PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 315
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready