fixed a race condition when getgrnam() which is not thread safe, could be used in threads
Costa Tsaousis (ktsaou) committed
Apr 15, 2016 at 03:18 UTC
1188686c7cc398d6a86fa8ee6704573d8aa050ad
5 files changed
+34
-17
src/daemon.c
+17
-14
@@ -82,9 +82,12 @@ int become_user(const char *username)
82
return -1;
83
}
84
85
- if(pidfile[0] && getuid() != pw->pw_uid) {
85
+ uid_t uid = pw->pw_uid;
86
+ gid_t gid = pw->pw_gid;
87
+
88
+ if(pidfile[0] && getuid() != uid) {
89
// we are dropping privileges
87
- if(chown(pidfile, pw->pw_uid, pw->pw_gid) != 0)
90
+ if(chown(pidfile, uid, gid) != 0)
91
error("Cannot chown pidfile '%s' to user '%s'", pidfile, username);
92
93
else if(pidfd != -1) {
@@ -99,30 +102,30 @@ int become_user(const char *username)
102
pidfd = -1;
103
}
104
102
- if(setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) != 0) {
103
- error("Cannot switch to user's %s group (gid: %d).", username, pw->pw_gid);
105
+ if(setresgid(gid, gid, gid) != 0) {
106
+ error("Cannot switch to user's %s group (gid: %d).", username, gid);
107
return -1;
108
}
109
107
- if(setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) != 0) {
108
- error("Cannot switch to user %s (uid: %d).", username, pw->pw_uid);
110
+ if(setresuid(uid, uid, uid) != 0) {
111
+ error("Cannot switch to user %s (uid: %d).", username, uid);
112
return -1;
113
}
114
112
- if(setgid(pw->pw_gid) != 0) {
113
- error("Cannot switch to user's %s group (gid: %d).", username, pw->pw_gid);
115
+ if(setgid(gid) != 0) {
116
+ error("Cannot switch to user's %s group (gid: %d).", username, gid);
117
return -1;
118
}
116
- if(setegid(pw->pw_gid) != 0) {
117
- error("Cannot effectively switch to user's %s group (gid: %d).", username, pw->pw_gid);
119
+ if(setegid(gid) != 0) {
120
+ error("Cannot effectively switch to user's %s group (gid: %d).", username, gid);
121
return -1;
122
}
120
- if(setuid(pw->pw_uid) != 0) {
121
- error("Cannot switch to user %s (uid: %d).", username, pw->pw_uid);
123
+ if(setuid(uid) != 0) {
124
+ error("Cannot switch to user %s (uid: %d).", username, uid);
125
return -1;
126
}
124
- if(seteuid(pw->pw_uid) != 0) {
125
- error("Cannot effectively switch to user %s (uid: %d).", username, pw->pw_uid);
127
+ if(seteuid(uid) != 0) {
128
+ error("Cannot effectively switch to user %s (uid: %d).", username, uid);
129
return -1;
130
}
131
src/main.c
+6
-1
@@ -420,8 +420,13 @@ int main(int argc, char **argv)
420
421
// --------------------------------------------------------------------
422
423
+ // get the user we should run
424
+ // IMPORTANT: this is required before web_files_uid()
425
user = config_get("global", "run as user" , (getuid() == 0)?NETDATA_USER:"");
424
- web_files_uid();
426
+
427
+ // IMPORTANT: these have to run once, while single threaded
428
+ web_files_uid(); // IMPORTANT: web_files_uid() before web_files_gid()
429
+ web_files_gid();
430
431
// --------------------------------------------------------------------
432
src/web_client.c
+9
-2
@@ -28,6 +28,7 @@
28
#include "rrd2json.h"
29
30
#include "web_client.h"
31
+#include "../config.h"
32
33
#define INITIAL_WEB_DATA_LENGTH 16384
34
#define WEB_REQUEST_LENGTH 16384
@@ -232,10 +233,13 @@ uid_t web_files_uid(void)
233
static uid_t owner_uid = 0;
234
235
if(unlikely(!web_owner)) {
235
- web_owner = config_get("global", "web files owner", NETDATA_USER);
236
+ web_owner = config_get("global", "web files owner", config_get("global", "run as user", ""));
237
if(!web_owner || !*web_owner)
238
owner_uid = geteuid();
239
else {
240
+ // getpwnam() is not thread safe,
241
+ // but we have called this function once
242
+ // while single threaded
243
struct passwd *pw = getpwnam(web_owner);
244
if(!pw) {
245
error("User %s is not present. Ignoring option.", web_owner);
@@ -257,10 +261,13 @@ gid_t web_files_gid(void)
261
static gid_t owner_gid = 0;
262
263
if(unlikely(!web_group)) {
260
- web_group = config_get("global", "web files group", config_get("global", "web files owner", NETDATA_USER));
264
+ web_group = config_get("global", "web files group", config_get("global", "web files owner", ""));
265
if(!web_group || !*web_group)
266
owner_gid = getegid();
267
else {
268
+ // getgrnam() is not thread safe,
269
+ // but we have called this function once
270
+ // while single threaded
271
struct group *gr = getgrnam(web_group);
272
if(!gr) {
273
error("Group %s is not present. Ignoring option.", web_group);
src/web_client.h
+1
@@ -81,6 +81,7 @@ struct web_client {
81
extern struct web_client *web_clients;
82
83
extern uid_t web_files_uid(void);
84
+extern uid_t web_files_gid(void);
85
86
extern struct web_client *web_client_create(int listener);
87
extern struct web_client *web_client_free(struct web_client *w);
src/web_server.c
+1
@@ -24,6 +24,7 @@
24
#include "global_statistics.h"
25
#include "rrd.h"
26
#include "rrd2json.h"
27
+#include "../config.h"
28
29
int listen_backlog = LISTEN_BACKLOG;
30