Skip to content

Commit 741495c

Browse files
tuomasjjrasanenyoe
authored andcommitted
nbd-server: handle modern-style negotiation in a child process
Previously, the modern style negotiation was carried out in the root server (listener) process before forking the actual client handler. This made it possible for a malfunctioning or evil client to terminate the root process simply by querying a non-existent export or aborting in the middle of the negotation process (caused SIGPIPE in the server). This commit moves the negotiation process to the child to keep the root process up and running no matter what happens during the negotiation. See http://sourceforge.net/mailarchive/message.php?msg_id=30410146 Signed-off-by: Tuomas Räsänen <tuomasjjrasanen@tjjr.fi>
1 parent 17fe191 commit 741495c

File tree

1 file changed

+157
-12
lines changed

1 file changed

+157
-12
lines changed

Diff for: nbd-server.c

+157-12
Original file line numberDiff line numberDiff line change
@@ -2198,6 +2198,161 @@ void destroy_pid_t(gpointer data) {
21982198
g_free(data);
21992199
}
22002200

2201+
static pid_t
2202+
spawn_child()
2203+
{
2204+
pid_t pid;
2205+
sigset_t newset;
2206+
sigset_t oldset;
2207+
2208+
sigemptyset(&newset);
2209+
sigaddset(&newset, SIGCHLD);
2210+
sigaddset(&newset, SIGTERM);
2211+
sigprocmask(SIG_BLOCK, &newset, &oldset);
2212+
pid = fork();
2213+
if (pid < 0) {
2214+
msg(LOG_ERR, "Could not fork (%s)", strerror(errno));
2215+
goto out;
2216+
}
2217+
if (pid > 0) { /* Parent */
2218+
pid_t *pidp;
2219+
2220+
pidp = g_malloc(sizeof(pid_t));
2221+
*pidp = pid;
2222+
g_hash_table_insert(children, pidp, pidp);
2223+
goto out;
2224+
}
2225+
/* Child */
2226+
signal(SIGCHLD, SIG_DFL);
2227+
signal(SIGTERM, SIG_DFL);
2228+
signal(SIGHUP, SIG_DFL);
2229+
out:
2230+
sigprocmask(SIG_SETMASK, &oldset, NULL);
2231+
return pid;
2232+
}
2233+
2234+
static int
2235+
socket_accept(const int sock)
2236+
{
2237+
struct sockaddr_storage addrin;
2238+
socklen_t addrinlen = sizeof(addrin);
2239+
int net;
2240+
2241+
net = accept(sock, (struct sockaddr *) &addrin, &addrinlen);
2242+
if (net < 0) {
2243+
err_nonfatal("Failed to accept socket connection: %m");
2244+
}
2245+
2246+
return net;
2247+
}
2248+
2249+
static void
2250+
handle_modern_connection(GArray *const servers, const int sock)
2251+
{
2252+
int net;
2253+
pid_t pid;
2254+
CLIENT *client = NULL;
2255+
int sock_flags_old;
2256+
int sock_flags_new;
2257+
2258+
net = socket_accept(sock);
2259+
if (net < 0)
2260+
return;
2261+
2262+
if (!dontfork) {
2263+
pid = spawn_child();
2264+
if (pid) {
2265+
if (pid > 0)
2266+
msg(LOG_INFO, "Spawned a child process");
2267+
if (pid < 0)
2268+
msg(LOG_ERR, "Failed to spawn a child process");
2269+
close(net);
2270+
return;
2271+
}
2272+
/* Child just continues. */
2273+
}
2274+
2275+
client = negotiate(net, NULL, servers, NEG_INIT | NEG_MODERN);
2276+
if (!client) {
2277+
msg(LOG_ERR, "Modern initial negotiation failed");
2278+
goto handler_err;
2279+
}
2280+
2281+
if (client->server->max_connections > 0 &&
2282+
g_hash_table_size(children) >= client->server->max_connections) {
2283+
msg(LOG_ERR, "Max connections (%d) reached",
2284+
client->server->max_connections);
2285+
goto handler_err;
2286+
}
2287+
2288+
sock_flags_old = fcntl(net, F_GETFL, 0);
2289+
if (sock_flags_old == -1) {
2290+
msg(LOG_ERR, "Failed to get socket flags");
2291+
goto handler_err;
2292+
}
2293+
2294+
sock_flags_new = sock_flags_old & ~O_NONBLOCK;
2295+
if (sock_flags_new != sock_flags_old &&
2296+
fcntl(net, F_SETFL, sock_flags_new) == -1) {
2297+
msg(LOG_ERR, "Failed to set socket to blocking mode");
2298+
goto handler_err;
2299+
}
2300+
2301+
if (set_peername(net, client)) {
2302+
msg(LOG_ERR, "Failed to set peername");
2303+
goto handler_err;
2304+
}
2305+
2306+
if (!authorized_client(client)) {
2307+
msg(LOG_INFO, "Client '%s' is not authorized to access",
2308+
client->clientname);
2309+
goto handler_err;
2310+
}
2311+
2312+
if (!dontfork) {
2313+
int i;
2314+
2315+
/* Free all root server resources here, because we are
2316+
* currently in the child process serving one specific
2317+
* connection. These are not simply needed anymore. */
2318+
g_hash_table_destroy(children);
2319+
children = NULL;
2320+
for (i = 0; i < modernsocks->len; i++) {
2321+
close(g_array_index(modernsocks, int, i));
2322+
}
2323+
g_array_free(modernsocks, TRUE);
2324+
2325+
/* Now that we are in the child process after a
2326+
* succesful negotiation, we do not need the list of
2327+
* servers anymore, get rid of it.*/
2328+
2329+
for (i = 0; i < servers->len; i++) {
2330+
const SERVER *const server = &g_array_index(servers, SERVER, i);
2331+
close(server->socket);
2332+
}
2333+
2334+
/* FALSE does not free the
2335+
actual data. This is required,
2336+
because the client has a
2337+
direct reference into that
2338+
data, and otherwise we get a
2339+
segfault... */
2340+
g_array_free(servers, FALSE);
2341+
}
2342+
2343+
msg(LOG_INFO, "Starting to serve");
2344+
serveconnection(client);
2345+
exit(EXIT_SUCCESS);
2346+
2347+
handler_err:
2348+
g_free(client);
2349+
close(net);
2350+
2351+
if (!dontfork) {
2352+
exit(EXIT_FAILURE);
2353+
}
2354+
}
2355+
22012356
static void
22022357
handle_connection(GArray *servers, int net, SERVER *serve, CLIENT *client)
22032358
{
@@ -2425,28 +2580,18 @@ void serveloop(GArray* servers) {
24252580

24262581
memcpy(&rset, &mset, sizeof(fd_set));
24272582
if(select(max+1, &rset, NULL, NULL, NULL)>0) {
2428-
int net;
24292583

24302584
DEBUG("accept, ");
24312585
for(i=0; i < modernsocks->len; i++) {
24322586
int sock = g_array_index(modernsocks, int, i);
24332587
if(!FD_ISSET(sock, &rset)) {
24342588
continue;
24352589
}
2436-
CLIENT *client;
24372590

2438-
if((net=accept(sock, (struct sockaddr *) &addrin, &addrinlen)) < 0) {
2439-
err_nonfatal("accept: %m");
2440-
continue;
2441-
}
2442-
client = negotiate(net, NULL, servers, NEG_INIT | NEG_MODERN);
2443-
if(!client) {
2444-
close(net);
2445-
continue;
2446-
}
2447-
handle_connection(servers, net, client->server, client);
2591+
handle_modern_connection(servers, sock);
24482592
}
24492593
for(i=0; i < servers->len; i++) {
2594+
int net;
24502595
SERVER *serve;
24512596

24522597
serve=&(g_array_index(servers, SERVER, i));

0 commit comments

Comments
 (0)