Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use threadsafe wrapper for getpwnam/getgrnam #775

Closed
wants to merge 1 commit into from

Conversation

ncopa
Copy link
Contributor

@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

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 FreeRADIUS#767
alandekok added a commit that referenced this pull request Sep 2, 2014
If the user is building threaded on a system without those functions,
too bad.  It's 2014, and every sane system has those functions
alandekok added a commit that referenced this pull request Sep 2, 2014
If the user is building threaded on a system without those functions,
too bad.  It's 2014, and every sane system has those functions
@alandekok alandekok closed this in cfdfd25 Sep 2, 2014
@ncopa
Copy link
Contributor Author

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
Copy link
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rlm_unix crashes due to thread-safety issues regardless of RLM_TYPE_THREAD_UNSAFE
2 participants