Skip to content

Commit

Permalink
CP-43755: Pam: avoid sleep(1) call when multithreaded
Browse files Browse the repository at this point in the history
Backport of 3b52b72

This enables PAM to be used in multithreaded mode (currently XAPI has a global lock around auth).

Using an off-cpu flamegraph I identified that concurrent PAM calls are slow due to a call to `sleep(1)`.
`pam_authenticate` calls `crypt_r` which calls `NSSLOW_Init` which on first use will try to initialize the just `dlopen`-ed library.
If it encounters a race condition it does a `sleep(1)`. This race condition can be quite reliably reproduced when performing a lot of PAM authentications from multiple threads in parallel.

GDB can also be used to confirm this by putting a breakpoint on `sleep`:
```
  #0  __sleep (seconds=seconds@entry=1) at ../sysdeps/unix/sysv/linux/sleep.c:42
  #1  0x00007ffff1548e22 in freebl_RunLoaderOnce () at lowhash_vector.c:122
  #2  0x00007ffff1548f31 in freebl_InitVector () at lowhash_vector.c:131
  #3  NSSLOW_Init () at lowhash_vector.c:148
  #4  0x00007ffff1b8f09a in __sha512_crypt_r (key=key@entry=0x7fffd8005a60 "pamtest-edvint", salt=0x7ffff31e17b8 "dIJbsXKc0",
  xapi-project#5  0x00007ffff1b8d070 in __crypt_r (key=key@entry=0x7fffd8005a60 "pamtest-edvint", salt=<optimized out>,
  xapi-project#6  0x00007ffff1dc9abc in verify_pwd_hash (p=p@entry=0x7fffd8005a60 "pamtest-edvint", hash=<optimized out>, nullok=nullok@entry=0) at passverify.c:111
  xapi-project#7  0x00007ffff1dc9139 in _unix_verify_password (pamh=pamh@entry=0x7fffd8002910, name=0x7fffd8002ab0 "pamtest-edvint", p=0x7fffd8005a60 "pamtest-edvint", ctrl=ctrl@entry=8389156) at support.c:777
  xapi-project#8  0x00007ffff1dc6556 in pam_sm_authenticate (pamh=0x7fffd8002910, flags=<optimized out>, argc=<optimized out>, argv=<optimized out>) at pam_unix_auth.c:178
  xapi-project#9  0x00007ffff7bcef1a in _pam_dispatch_aux (use_cached_chain=<optimized out>, resumed=<optimized out>, h=<optimized out>, flags=1, pamh=0x7fffd8002910) at pam_dispatch.c:110
  xapi-project#10 _pam_dispatch (pamh=pamh@entry=0x7fffd8002910, flags=1, choice=choice@entry=1) at pam_dispatch.c:426
  xapi-project#11 0x00007ffff7bce7e0 in pam_authenticate (pamh=0x7fffd8002910, flags=flags@entry=1) at pam_auth.c:34
  xapi-project#12 0x00000000005ae567 in XA_mh_authorize (username=username@entry=0x7fffd80028d0 "pamtest-edvint", password=password@entry=0x7fffd80028f0 "pamtest-edvint", error=error@entry=0x7ffff31e1be8) at xa_auth.c:83
  xapi-project#13 0x00000000005adf20 in stub_XA_mh_authorize (username=<optimized out>, password=<optimized out>) at xa_auth_stubs.c:42
```

`pam_start` and `pam_end` doesn't help here, because on `pam_end` the library is `dlclose`-ed, so on next `pam_authenticate` it will have to go through the initialization code again.
(This initialization code would've belonged into `pam_start`, not `pam_authenticate`, but there are several layers here including a call to `crypt_r`).
Upstream has fixed this problem >5 years ago by switching to libxcrypt instead.

Signed-off-by: Edwin Török <[email protected]>
Signed-off-by: Christian Lindig <[email protected]>
  • Loading branch information
edwintorok authored and lindig committed Feb 29, 2024
1 parent 9601578 commit 8f355ea
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 3 deletions.
2 changes: 1 addition & 1 deletion ocaml/auth/dune
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
xa_auth_stubs
)
(c_flags -I/usr/lib64/ocaml)
(c_library_flags -lpam)
(c_library_flags -lpam -lcrypt)
(wrapped false)
)
29 changes: 27 additions & 2 deletions ocaml/auth/xa_auth_stubs.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*/
/*
*/

/* must be at the beginning, it affects defines in other headers that cannot be reenabled later */
#define _GNU_SOURCE

#include <unistd.h>
#include <stdlib.h>
Expand Down Expand Up @@ -72,6 +73,30 @@ CAMLprim value stub_XA_mh_chpasswd(value username, value new_password){
CAMLreturn(ret);
}

#include <crypt.h>
/* 'constructor' attribute will ensure this function gets call early during program startup. */
void __attribute__((constructor)) stub_XA_workaround(void)
{
struct crypt_data data;
memset(&data, 0, sizeof(data));

/* Initialize and load crypt library used for password hashing.
This library is loaded and initialized at [pam_authenticate] time and not at [pam_start].
If it detects a race condition (multiple threads with their own PAM contexts trying to call 'crypt_r'),
then it does [sleep 1] as can be seen in this call trace:
[pam_authenticate -> crypt_r -> __sha512_crypt_r -> freebl_InitVector -> freebl_RunLoaderOnce -> sleep].
As a workaround link with 'libcrypt' and call 'crypt_r' with a setting that will make it take tha sha512 route
to ensure that the library gets initialized while we're still single-threaded and stays loaded and initialized.
'$6$' is the setting for sha512 according to crypt(5).
Upstream has switched to using libxcrypt instead which doesn't have these problems, when we switch then
this workaround can be dropped.
*/
crypt_r("", "$6$", &data);
}

/*
* Local variables:
* mode: C
Expand Down

0 comments on commit 8f355ea

Please sign in to comment.