Skip to content

Commit

Permalink
Use getpwuid_r() instead of getpwuid()
Browse files Browse the repository at this point in the history
In the Netopeer2 test suite, two threads were calling getpwuid(), which
is not a reentrant function. Here's a trace from TSAN:

WARNING: ThreadSanitizer: data race (pid=3803971)
  Write of size 8 at 0x7fc95e073f40 by thread T2:
    #0 getpwuid <null> (test_parallel_sessions+0x4367d4)
    #1 nc_connect_unix libnetconf2/src/session_client.c:1270:10 (libnetconf2.so.2+0x1ba34)
    #2 send_get_rpc Netopeer2/tests/test_parallel_sessions.c:68:15 (test_parallel_sessions+0x4ea807)

  Previous write of size 8 at 0x7fc95e073f40 by thread T1:
    #0 getpwuid <null> (test_parallel_sessions+0x4367d4)
    #1 nc_connect_unix libnetconf2/src/session_client.c:1270:10 (libnetconf2.so.2+0x1ba34)
    #2 send_get_rpc Netopeer2/tests/test_parallel_sessions.c:68:15 (test_parallel_sessions+0x4ea807)

  Location is global 'resbuf.11357' of size 48 at 0x7fc95e073f40 (libc.so.6+0x1bcf40)

  Thread T2 (tid=3807449, running) created by main thread at:
    #0 pthread_create <null> (test_parallel_sessions+0x469032)
    #1 test_first Netopeer2/tests/test_parallel_sessions.c:115:9 (test_parallel_sessions+0x4ea6c2)
    #2 cmocka_run_one_test_or_fixture <null> (libcmocka.so.0+0x630e)

  Thread T1 (tid=3807448, running) created by main thread at:
    #0 pthread_create <null> (test_parallel_sessions+0x469032)
    #1 test_first Netopeer2/tests/test_parallel_sessions.c:115:9 (test_parallel_sessions+0x4ea6ad)
    #2 cmocka_run_one_test_or_fixture <null> (libcmocka.so.0+0x630e)

SUMMARY: ThreadSanitizer: data race (Netopeer2/tests/test_parallel_sessions+0x4367d4) in __interceptor_getpwuid

Fix that by switching to getpwuid_r(). The code is very similar to the
auth_password_getpwnam() wrapper from session_server_ssh.c. I chose to
put this into io.c even though it is not really an IO function, but the
existing nc_realloc() is in that file as well.
  • Loading branch information
jktjkt authored and michalvasko committed Oct 11, 2021
1 parent aa32310 commit 6aa0eeb
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 10 deletions.
38 changes: 38 additions & 0 deletions src/io.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@
#include <errno.h>
#include <inttypes.h>
#include <poll.h>
#include <pwd.h>
#include <signal.h>
#include <stdarg.h>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <time.h>
#include <unistd.h>

Expand Down Expand Up @@ -1158,3 +1160,39 @@ nc_realloc(void *ptr, size_t size)

return ret;
}

struct passwd *
nc_getpwuid(uid_t uid, struct passwd *pwd_buf, char **buf, size_t *buf_size)
{
struct passwd *pwd = NULL;
char *mem;
int r = 0;

do {
r = getpwuid_r(uid, pwd_buf, *buf, *buf_size, &pwd);
if (pwd) {
break;
}

if (r == ERANGE) {
if (!*buf_size) {
ssize_t size = sysconf(_SC_GETPW_R_SIZE_MAX);
if (size == -1) {
*buf_size = 256;
} else {
*buf_size = size;
}
} else {
*buf_size <<= 2;
}
mem = realloc(*buf, *buf_size);
if (!mem) {
ERRMEM;
return NULL;
}
*buf = mem;
}
} while (r == ERANGE);

return pwd;
}
6 changes: 4 additions & 2 deletions src/session_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -1238,16 +1238,18 @@ nc_connect_unix(const char *address, struct ly_ctx *ctx)
{
struct nc_session *session = NULL;
struct sockaddr_un sun;
const struct passwd *pw;
struct passwd *pw, pw_buf;
char *username;
int sock = -1;
char *buf = NULL;
size_t buf_size = 0;

if (address == NULL) {
ERRARG("address");
return NULL;
}

pw = getpwuid(geteuid());
pw = nc_getpwuid(geteuid(), &pw_buf, &buf, &buf_size);
if (pw == NULL) {
ERR(NULL, "Failed to find username for euid=%u.\n", geteuid());
goto fail;
Expand Down
18 changes: 12 additions & 6 deletions src/session_client_ssh.c
Original file line number Diff line number Diff line change
Expand Up @@ -1545,8 +1545,10 @@ _nc_connect_libssh(ssh_session ssh_session, struct ly_ctx *ctx, struct nc_keepal
char *host = NULL, *username = NULL, *ip_host;
unsigned int port = 0;
int sock;
struct passwd *pw;
struct passwd *pw, pw_buf;
struct nc_session *session = NULL;
char *buf;
size_t buf_len;

if (!ssh_session) {
ERRARG("ssh_session");
Expand Down Expand Up @@ -1604,7 +1606,7 @@ _nc_connect_libssh(ssh_session ssh_session, struct ly_ctx *ctx, struct nc_keepal
/* remember username */
if (!username) {
if (!opts->username) {
pw = getpwuid(getuid());
pw = nc_getpwuid(getuid(), &pw_buf, &buf, &buf_len);
if (!pw) {
ERR(NULL, "Unknown username for the SSH connection (%s).", strerror(errno));
goto fail;
Expand Down Expand Up @@ -1683,8 +1685,10 @@ nc_connect_ssh(const char *host, uint16_t port, struct ly_ctx *ctx)
int sock;
uint32_t port_uint;
char *username, *ip_host = NULL;
struct passwd *pw;
struct passwd *pw, pw_buf;
struct nc_session *session = NULL;
char *buf = NULL;
size_t buf_len = 0;

/* process parameters */
if (!host || strisempty(host)) {
Expand All @@ -1697,7 +1701,7 @@ nc_connect_ssh(const char *host, uint16_t port, struct ly_ctx *ctx)
port_uint = port;

if (!ssh_opts.username) {
pw = getpwuid(getuid());
pw = nc_getpwuid(getuid(), &pw_buf, &buf, &buf_len);
if (!pw) {
ERR(session, "Unknown username for the SSH connection (%s).", strerror(errno));
return NULL;
Expand Down Expand Up @@ -1855,9 +1859,11 @@ nc_accept_callhome_ssh_sock(int sock, const char *host, uint16_t port, struct ly
{
const long ssh_timeout = NC_SSH_TIMEOUT;
unsigned int uint_port;
struct passwd *pw;
struct passwd *pw, pw_buf;
struct nc_session *session;
ssh_session sess;
char *buf = NULL;
size_t buf_len = 0;

sess = ssh_new();
if (!sess) {
Expand All @@ -1873,7 +1879,7 @@ nc_accept_callhome_ssh_sock(int sock, const char *host, uint16_t port, struct ly
ssh_options_set(sess, SSH_OPTIONS_PORT, &uint_port);
ssh_options_set(sess, SSH_OPTIONS_TIMEOUT, &ssh_timeout);
if (!ssh_ch_opts.username) {
pw = getpwuid(getuid());
pw = nc_getpwuid(getuid(), &pw_buf, &buf, &buf_len);
if (!pw) {
ERR(NULL, "Unknown username for the SSH connection (%s).", strerror(errno));
ssh_free(sess);
Expand Down
2 changes: 2 additions & 0 deletions src/session_p.h
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,8 @@ struct nc_ntf_thread_arg {

void *nc_realloc(void *ptr, size_t size);

struct passwd *nc_getpwuid(uid_t uid, struct passwd *pwd_buf, char **buf, size_t *buf_size);

NC_MSG_TYPE nc_send_msg_io(struct nc_session *session, int io_timeout, struct lyd_node *op);

int nc_gettimespec_mono(struct timespec *ts);
Expand Down
6 changes: 4 additions & 2 deletions src/session_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -1877,17 +1877,19 @@ static int
nc_accept_unix(struct nc_session *session, int sock)
{
#if defined (SO_PEERCRED) || defined (HAVE_GETPEEREID)
const struct passwd *pw;
struct passwd *pw, pw_buf;
char *username;
session->ti_type = NC_TI_UNIX;
uid_t uid = 0;
char *buf = NULL;
size_t buf_len = 0;

if (nc_get_uid(sock, &uid)) {
close(sock);
return -1;
}

pw = getpwuid(uid);
pw = nc_getpwuid(uid, &pw_buf, &buf, &buf_len);
if (pw == NULL) {
ERR(NULL, "Failed to find username for uid=%u (%s).\n", uid, strerror(errno));
close(sock);
Expand Down

0 comments on commit 6aa0eeb

Please sign in to comment.