Skip to content

Commit 54e80f1

Browse files
kolyshkinkolyshkin
committed
libct/nsenter: better read/write errors
Introduce and use CHECK_IO and CHECK_IO_KILL macros so that we can call either bail or bailx on error, depending on read/write return. This prevents the "Success" prefix in errors like: failed to sync with stage-1: next state: Success Signed-off-by: Kir Kolyshkin <[email protected]>
1 parent 9af7aea commit 54e80f1

File tree

1 file changed

+63
-65
lines changed

1 file changed

+63
-65
lines changed

‎libcontainer/nsenter/nsexec.c‎

Lines changed: 63 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,36 @@ int setns(int fd, int nstype)
132132
}
133133
#endif
134134

135+
/*
136+
* CHECK_IO calls op (read or write) and checks its return value.
137+
* If a syscall returns -1 (error), it calls bail() to report errno.
138+
* Otherwise, check for a short read/write and call bailx() on error.
139+
*/
140+
#define CHECK_IO(op, fd, buf, count, ...) \
141+
do { \
142+
ssize_t __ret = op(fd, buf, count); \
143+
if (__ret < 0) \
144+
bail(__VA_ARGS__); \
145+
if (__ret != (ssize_t)(count)) \
146+
bailx(__VA_ARGS__ ": short " #op); \
147+
} while (0)
148+
149+
/*
150+
* CHECK_IO_KILL is a variant of CHECK_IO that kills PIDs before bailing.
151+
* Use this when you need to kill child process(es) on I/O failure.
152+
*/
153+
#define CHECK_IO_KILL(op, fd, buf, count, pid1, pid2, ...) \
154+
do { \
155+
ssize_t __ret = op(fd, buf, count); \
156+
if (__ret != (ssize_t)(count)) { \
157+
sane_kill(pid1, SIGKILL); \
158+
sane_kill(pid2, SIGKILL); \
159+
if (__ret < 0) \
160+
bail(__VA_ARGS__); \
161+
bailx(__VA_ARGS__ ": short " #op); \
162+
} \
163+
} while (0)
164+
135165
/* XXX: This is ugly. */
136166
static int syncfd = -1;
137167

@@ -334,16 +364,12 @@ static uint8_t readint8(char *buf)
334364

335365
static void nl_parse(int fd, struct nlconfig_t *config)
336366
{
337-
size_t len, size;
367+
size_t size;
338368
struct nlmsghdr hdr;
339369
char *data, *current;
340370

341371
/* Retrieve the netlink header. */
342-
len = read(fd, &hdr, NLMSG_HDRLEN);
343-
if (len < 0)
344-
bail("failed to read netlink header");
345-
if (len != NLMSG_HDRLEN)
346-
bailx("invalid netlink header length %zu", len);
372+
CHECK_IO(read, fd, &hdr, NLMSG_HDRLEN, "failed to read netlink header");
347373

348374
if (hdr.nlmsg_type == NLMSG_ERROR)
349375
bailx("failed to read netlink message");
@@ -357,11 +383,7 @@ static void nl_parse(int fd, struct nlconfig_t *config)
357383
if (!data)
358384
bail("failed to allocate %zu bytes of memory for nl_payload", size);
359385

360-
len = read(fd, data, size);
361-
if (len < 0)
362-
bail("failed to read netlink payload");
363-
if (len != size)
364-
bailx("failed to read netlink payload, %zu != %zu", len, size);
386+
CHECK_IO(read, fd, data, size, "failed to read netlink payload");
365387

366388
/* Parse the netlink payload. */
367389
config->data = data;
@@ -863,8 +885,7 @@ void nsexec(void)
863885
while (!stage1_complete) {
864886
enum sync_t s;
865887

866-
if (read(syncfd, &s, sizeof(s)) != sizeof(s))
867-
bail("failed to sync with stage-1: next state");
888+
CHECK_IO(read, syncfd, &s, sizeof(s), "failed to sync with stage-1: next state");
868889

869890
switch (s) {
870891
case SYNC_USERMAP_PLS:
@@ -888,28 +909,20 @@ void nsexec(void)
888909
update_gidmap(config.gidmappath, stage1_pid, config.gidmap, config.gidmap_len);
889910

890911
s = SYNC_USERMAP_ACK;
891-
if (write(syncfd, &s, sizeof(s)) != sizeof(s)) {
892-
sane_kill(stage1_pid, SIGKILL);
893-
sane_kill(stage2_pid, SIGKILL);
894-
bail("failed to sync with stage-1: write(SYNC_USERMAP_ACK)");
895-
}
912+
CHECK_IO_KILL(write, syncfd, &s, sizeof(s), stage1_pid, stage2_pid,
913+
"failed to sync with stage-1: write(SYNC_USERMAP_ACK)");
896914
break;
897915
case SYNC_RECVPID_PLS:
898916
write_log(DEBUG, "stage-1 requested pid to be forwarded");
899917

900918
/* Get the stage-2 pid. */
901-
if (read(syncfd, &stage2_pid, sizeof(stage2_pid)) != sizeof(stage2_pid)) {
902-
sane_kill(stage1_pid, SIGKILL);
903-
bail("failed to sync with stage-1: read(stage2_pid)");
904-
}
919+
CHECK_IO_KILL(read, syncfd, &stage2_pid, sizeof(stage2_pid), stage1_pid, -1,
920+
"failed to sync with stage-1: read(stage2_pid)");
905921

906922
/* Send ACK. */
907923
s = SYNC_RECVPID_ACK;
908-
if (write(syncfd, &s, sizeof(s)) != sizeof(s)) {
909-
sane_kill(stage1_pid, SIGKILL);
910-
sane_kill(stage2_pid, SIGKILL);
911-
bail("failed to sync with stage-1: write(SYNC_RECVPID_ACK)");
912-
}
924+
CHECK_IO_KILL(write, syncfd, &s, sizeof(s), stage1_pid, stage2_pid,
925+
"failed to sync with stage-1: write(SYNC_RECVPID_ACK)");
913926

914927
/*
915928
* Send both the stage-1 and stage-2 pids back to runc.
@@ -933,10 +946,8 @@ void nsexec(void)
933946
write_log(DEBUG, "stage-1 requested timens offsets to be configured");
934947
update_timens_offsets(stage1_pid, config.timensoffset, config.timensoffset_len);
935948
s = SYNC_TIMEOFFSETS_ACK;
936-
if (write(syncfd, &s, sizeof(s)) != sizeof(s)) {
937-
sane_kill(stage1_pid, SIGKILL);
938-
bail("failed to sync with child: write(SYNC_TIMEOFFSETS_ACK)");
939-
}
949+
CHECK_IO_KILL(write, syncfd, &s, sizeof(s), stage1_pid, -1,
950+
"failed to sync with child: write(SYNC_TIMEOFFSETS_ACK)");
940951
break;
941952
case SYNC_CHILD_FINISH:
942953
write_log(DEBUG, "stage-1 complete");
@@ -960,13 +971,10 @@ void nsexec(void)
960971

961972
write_log(DEBUG, "signalling stage-2 to run");
962973
s = SYNC_GRANDCHILD;
963-
if (write(syncfd, &s, sizeof(s)) != sizeof(s)) {
964-
sane_kill(stage2_pid, SIGKILL);
965-
bail("failed to sync with child: write(SYNC_GRANDCHILD)");
966-
}
974+
CHECK_IO_KILL(write, syncfd, &s, sizeof(s), stage2_pid, -1,
975+
"failed to sync with child: write(SYNC_GRANDCHILD)");
967976

968-
if (read(syncfd, &s, sizeof(s)) != sizeof(s))
969-
bail("failed to sync with child: next state");
977+
CHECK_IO(read, syncfd, &s, sizeof(s), "failed to sync with child: next state");
970978

971979
switch (s) {
972980
case SYNC_CHILD_FINISH:
@@ -1057,13 +1065,13 @@ void nsexec(void)
10571065
*/
10581066
write_log(DEBUG, "request stage-0 to map user namespace");
10591067
s = SYNC_USERMAP_PLS;
1060-
if (write(syncfd, &s, sizeof(s)) != sizeof(s))
1061-
bail("failed to sync with parent: write(SYNC_USERMAP_PLS)");
1068+
CHECK_IO(write, syncfd, &s, sizeof(s),
1069+
"failed to sync with parent: write(SYNC_USERMAP_PLS)");
10621070

10631071
/* ... wait for mapping ... */
10641072
write_log(DEBUG, "waiting stage-0 to complete the mapping of user namespace");
1065-
if (read(syncfd, &s, sizeof(s)) != sizeof(s))
1066-
bail("failed to sync with parent: read(SYNC_USERMAP_ACK)");
1073+
CHECK_IO(read, syncfd, &s, sizeof(s),
1074+
"failed to sync with parent: read(SYNC_USERMAP_ACK)");
10671075
if (s != SYNC_USERMAP_ACK)
10681076
bailx("failed to sync with parent: SYNC_USERMAP_ACK: got %u", s);
10691077

@@ -1095,11 +1103,11 @@ void nsexec(void)
10951103
write_log(DEBUG, "request stage-0 to write timens offsets");
10961104

10971105
s = SYNC_TIMEOFFSETS_PLS;
1098-
if (write(syncfd, &s, sizeof(s)) != sizeof(s))
1099-
bail("failed to sync with parent: write(SYNC_TIMEOFFSETS_PLS)");
1106+
CHECK_IO(write, syncfd, &s, sizeof(s),
1107+
"failed to sync with parent: write(SYNC_TIMEOFFSETS_PLS)");
11001108

1101-
if (read(syncfd, &s, sizeof(s)) != sizeof(s))
1102-
bail("failed to sync with parent: read(SYNC_TIMEOFFSETS_ACK)");
1109+
CHECK_IO(read, syncfd, &s, sizeof(s),
1110+
"failed to sync with parent: read(SYNC_TIMEOFFSETS_ACK)");
11031111
if (s != SYNC_TIMEOFFSETS_ACK)
11041112
bailx("failed to sync with parent: SYNC_TIMEOFFSETS_ACK: got %u", s);
11051113
}
@@ -1121,31 +1129,23 @@ void nsexec(void)
11211129
/* Send the child to our parent, which knows what it's doing. */
11221130
write_log(DEBUG, "request stage-0 to forward stage-2 pid (%d)", stage2_pid);
11231131
s = SYNC_RECVPID_PLS;
1124-
if (write(syncfd, &s, sizeof(s)) != sizeof(s)) {
1125-
sane_kill(stage2_pid, SIGKILL);
1126-
bail("failed to sync with parent: write(SYNC_RECVPID_PLS)");
1127-
}
1128-
if (write(syncfd, &stage2_pid, sizeof(stage2_pid)) != sizeof(stage2_pid)) {
1129-
sane_kill(stage2_pid, SIGKILL);
1130-
bail("failed to sync with parent: write(stage2_pid)");
1131-
}
1132+
CHECK_IO_KILL(write, syncfd, &s, sizeof(s), stage2_pid, -1,
1133+
"failed to sync with parent: write(SYNC_RECVPID_PLS)");
1134+
CHECK_IO_KILL(write, syncfd, &stage2_pid, sizeof(stage2_pid), stage2_pid, -1,
1135+
"failed to sync with parent: write(stage2_pid)");
11321136

11331137
/* ... wait for parent to get the pid ... */
1134-
if (read(syncfd, &s, sizeof(s)) != sizeof(s)) {
1135-
sane_kill(stage2_pid, SIGKILL);
1136-
bail("failed to sync with parent: read(SYNC_RECVPID_ACK)");
1137-
}
1138+
CHECK_IO_KILL(read, syncfd, &s, sizeof(s), stage2_pid, -1,
1139+
"failed to sync with parent: read(SYNC_RECVPID_ACK)");
11381140
if (s != SYNC_RECVPID_ACK) {
11391141
sane_kill(stage2_pid, SIGKILL);
11401142
bailx("failed to sync with parent: SYNC_RECVPID_ACK: got %u", s);
11411143
}
11421144

11431145
write_log(DEBUG, "signal completion to stage-0");
11441146
s = SYNC_CHILD_FINISH;
1145-
if (write(syncfd, &s, sizeof(s)) != sizeof(s)) {
1146-
sane_kill(stage2_pid, SIGKILL);
1147-
bail("failed to sync with parent: write(SYNC_CHILD_FINISH)");
1148-
}
1147+
CHECK_IO_KILL(write, syncfd, &s, sizeof(s), stage2_pid, -1,
1148+
"failed to sync with parent: write(SYNC_CHILD_FINISH)");
11491149

11501150
/* Our work is done. [Stage 2: STAGE_INIT] is doing the rest of the work. */
11511151
write_log(DEBUG, "<~ nsexec stage-1");
@@ -1181,8 +1181,7 @@ void nsexec(void)
11811181
prctl(PR_SET_NAME, (unsigned long)"runc:[2:INIT]", 0, 0, 0);
11821182
write_log(DEBUG, "~> nsexec stage-2");
11831183

1184-
if (read(syncfd, &s, sizeof(s)) != sizeof(s))
1185-
bail("failed to sync with parent: read(SYNC_GRANDCHILD)");
1184+
CHECK_IO(read, syncfd, &s, sizeof(s), "failed to sync with parent: read(SYNC_GRANDCHILD)");
11861185
if (s != SYNC_GRANDCHILD)
11871186
bailx("failed to sync with parent: SYNC_GRANDCHILD: got %u", s);
11881187

@@ -1202,8 +1201,7 @@ void nsexec(void)
12021201

12031202
write_log(DEBUG, "signal completion to stage-0");
12041203
s = SYNC_CHILD_FINISH;
1205-
if (write(syncfd, &s, sizeof(s)) != sizeof(s))
1206-
bail("failed to sync with parent: write(SYNC_CHILD_FINISH)");
1204+
CHECK_IO(write, syncfd, &s, sizeof(s), "failed to sync with parent: write(SYNC_CHILD_FINISH)");
12071205

12081206
/* Close sync pipes. */
12091207
if (close(sync_grandchild_pipe[0]) < 0)

0 commit comments

Comments
 (0)