Use threadsafe wrapper for getpwnam/getgrnam #775

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@ncopa
ncopa commented Sep 2, 2014

Even if rlm_unix is marked as RLM_TYPE_THREAD_UNSAFE, it runs in a
separate thread than the main thread. Both main thread and rlm_unix
uses thread unsafe getpwnam/getgrnam which causes segfault when under
stress.

We create a thread safe wrapper for those that uses TLS.

This fixes #767

@ncopa ncopa Use threadsafe wrapper for getpwnam/getgrnam
Even if rlm_unix is marked as RLM_TYPE_THREAD_UNSAFE, it runs in a
separate thread than the main thread. Both main thread and rlm_unix
uses thread unsafe getpwnam/getgrnam which causes segfault when under
stress.

We create a thread safe wrapper for those that uses TLS.

This fixes #767
de01b7f
@alandekok alandekok added a commit that referenced this pull request Sep 2, 2014
@alandekok alandekok Use getpwnam_r() and getgrnam_r() if available. Closes #775.
If the user is building threaded on a system without those functions,
too bad.  It's 2014, and every sane system has those functions
e845e01
@alandekok alandekok added a commit that referenced this pull request Sep 2, 2014
@alandekok alandekok Use getpwnam_r() and getgrnam_r() if available. Closes #775.
If the user is building threaded on a system without those functions,
too bad.  It's 2014, and every sane system has those functions
8227043
@alandekok alandekok added a commit that closed this pull request Sep 2, 2014
@alandekok alandekok Use getpwnam_r() and getgrnam_r() if available. Closes #775.
If the user is building threaded on a system without those functions,
too bad.  It's 2014, and every sane system has those functions
cfdfd25
@alandekok alandekok closed this in cfdfd25 Sep 2, 2014
@ncopa
ncopa commented Sep 2, 2014

Thanks for fixing the issue, but you have introduced a new by using a fixed sized buffer. You should really check for ERANGE and increase the buffer if needed.

I have had similar issue before but at that time the lower limit was in uclibc and busybox. And yes, I got surprised that people actually have that big groups in /etc/group and don't in ldap/sql whatever. Since then we have moved to musl libc so I suspect it is just a question of time before someone hits the 1024 char roof.

I'd prefer fix things properly while at it.

@alandekok
Member

ncopa wrote:

Thanks for fixing the issue, but you have introduced a new by using a
fixed sized buffer. You should really check for ERANGE and increase the
buffer if needed.

Hmm... OK.

I have had similar issue http://bugs.alpinelinux.org/issues/733 before
but at that time the lower limit was in uclibc and busybox. And yes, I
got surprised that people actually have that big groups in /etc/group
and don't in ldap/sql whatever. Since then we have moved to musl libc so
I suspect it is just a question of time before someone hits the 1024
char roof.

I'd prefer fix things properly while at it.

That's reasonable.

Alan DeKok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment