summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--daemon/command.c2
-rw-r--r--daemon/daemon.h26
-rw-r--r--daemon/df.c4
-rw-r--r--daemon/inotify.c2
-rw-r--r--daemon/mount.c8
-rw-r--r--daemon/proto.c6
-rw-r--r--daemon/tar.c34
-rw-r--r--daemon/upload.c13
-rwxr-xr-xregressions/rhbz576879.sh12
-rwxr-xr-xsrc/generator.ml18
10 files changed, 71 insertions, 54 deletions
diff --git a/daemon/command.c b/daemon/command.c
index 5e870673..48bed2de 100644
--- a/daemon/command.c
+++ b/daemon/command.c
@@ -36,7 +36,7 @@ do_command (char *const *argv)
int dev_ok, dev_pts_ok, proc_ok, selinux_ok, sys_ok;
/* We need a root filesystem mounted to do this. */
- NEED_ROOT (, return NULL);
+ NEED_ROOT (0, return NULL);
/* Conveniently, argv is already a NULL-terminated argv-style array
* of parameters, so we can pass it straight in to our internal
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 977dfdcb..de598cd3 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -139,10 +139,13 @@ typedef int (*receive_cb) (void *opaque, const void *buf, size_t len);
extern int receive_file (receive_cb cb, void *opaque);
/* daemon functions that receive files (FileIn) can call this
- * to cancel incoming transfers (eg. if there is a local error),
- * but they MUST then call reply_with_*.
+ * to cancel incoming transfers (eg. if there is a local error).
+ *
+ * If and only if this function does NOT return -2, they MUST then
+ * call reply_with_*
+ * (see https://bugzilla.redhat.com/show_bug.cgi?id=576879#c5).
*/
-extern void cancel_receive (void);
+extern int cancel_receive (void);
/* daemon functions that return files (FileOut) should call
* reply, then send_file_* for each FileOut parameter.
@@ -160,8 +163,8 @@ extern void reply (xdrproc_t xdrp, char *ret);
#define NEED_ROOT(cancel_stmt,fail_stmt) \
do { \
if (!root_mounted) { \
- cancel_stmt; \
- reply_with_error ("%s: you must call 'mount' first to mount the root filesystem", __func__); \
+ if ((cancel_stmt) != -2) \
+ reply_with_error ("%s: you must call 'mount' first to mount the root filesystem", __func__); \
fail_stmt; \
} \
} \
@@ -173,8 +176,8 @@ extern void reply (xdrproc_t xdrp, char *ret);
#define ABS_PATH(path,cancel_stmt,fail_stmt) \
do { \
if ((path)[0] != '/') { \
- cancel_stmt; \
- reply_with_error ("%s: path must start with a / character", __func__); \
+ if ((cancel_stmt) != -2) \
+ reply_with_error ("%s: path must start with a / character", __func__); \
fail_stmt; \
} \
} while (0)
@@ -189,15 +192,16 @@ extern void reply (xdrproc_t xdrp, char *ret);
#define RESOLVE_DEVICE(path,cancel_stmt,fail_stmt) \
do { \
if (STRNEQLEN ((path), "/dev/", 5)) { \
- cancel_stmt; \
- reply_with_error ("%s: %s: expecting a device name", __func__, (path)); \
+ if ((cancel_stmt) != -2) \
+ reply_with_error ("%s: %s: expecting a device name", __func__, (path)); \
fail_stmt; \
} \
if (device_name_translation ((path)) == -1) { \
int err = errno; \
- cancel_stmt; \
+ int r = cancel_stmt; \
errno = err; \
- reply_with_perror ("%s: %s", __func__, path); \
+ if (r != -2) \
+ reply_with_perror ("%s: %s", __func__, path); \
fail_stmt; \
} \
} while (0)
diff --git a/daemon/df.c b/daemon/df.c
index 7aa6f4fd..cce41a0e 100644
--- a/daemon/df.c
+++ b/daemon/df.c
@@ -33,7 +33,7 @@ do_df (void)
int r;
char *out, *err;
- NEED_ROOT (, return NULL);
+ NEED_ROOT (0, return NULL);
r = command (&out, &err, "df", NULL);
if (r == -1) {
@@ -54,7 +54,7 @@ do_df_h (void)
int r;
char *out, *err;
- NEED_ROOT (, return NULL);
+ NEED_ROOT (0, return NULL);
r = command (&out, &err, "df", "-h", NULL);
if (r == -1) {
diff --git a/daemon/inotify.c b/daemon/inotify.c
index 104fa60b..d5a5a73e 100644
--- a/daemon/inotify.c
+++ b/daemon/inotify.c
@@ -70,7 +70,7 @@ do_inotify_init (int max_events)
#ifdef HAVE_SYS_INOTIFY_H
FILE *fp;
- NEED_ROOT (, return -1);
+ NEED_ROOT (0, return -1);
if (max_events < 0) {
reply_with_error ("max_events < 0");
diff --git a/daemon/mount.c b/daemon/mount.c
index 8927c6c2..54851168 100644
--- a/daemon/mount.c
+++ b/daemon/mount.c
@@ -48,7 +48,7 @@ do_mount_vfs (const char *options, const char *vfstype,
char *mp;
char *error;
- ABS_PATH (mountpoint, , return -1);
+ ABS_PATH (mountpoint, 0, return -1);
is_root = STREQ (mountpoint, "/");
@@ -121,7 +121,7 @@ do_umount (const char *pathordevice)
}
if (is_dev)
- RESOLVE_DEVICE (buf, , { free (buf); return -1; });
+ RESOLVE_DEVICE (buf, 0, { free (buf); return -1; });
r = command (NULL, &err, "umount", buf, NULL);
free (buf);
@@ -356,7 +356,7 @@ do_mkmountpoint (const char *path)
int r;
/* NEED_ROOT (return -1); - we don't want this test for this call. */
- ABS_PATH (path, , return -1);
+ ABS_PATH (path, 0, return -1);
CHROOT_IN;
r = mkdir (path, 0777);
@@ -381,7 +381,7 @@ do_rmmountpoint (const char *path)
int r;
/* NEED_ROOT (return -1); - we don't want this test for this call. */
- ABS_PATH (path, , return -1);
+ ABS_PATH (path, 0, return -1);
CHROOT_IN;
r = rmdir (path);
diff --git a/daemon/proto.c b/daemon/proto.c
index ee1c400d..6fa243ff 100644
--- a/daemon/proto.c
+++ b/daemon/proto.c
@@ -395,7 +395,7 @@ receive_file (receive_cb cb, void *opaque)
}
/* Send a cancellation flag back to the library. */
-void
+int
cancel_receive (void)
{
XDR xdr;
@@ -408,11 +408,11 @@ cancel_receive (void)
if (xwrite (sock, fbuf, sizeof fbuf) == -1) {
perror ("write to socket");
- return;
+ return -1;
}
/* Keep receiving chunks and discarding, until library sees cancel. */
- (void) receive_file (NULL, NULL);
+ return receive_file (NULL, NULL);
}
static int check_for_library_cancellation (void);
diff --git a/daemon/tar.c b/daemon/tar.c
index 26a0d302..3f5f60a6 100644
--- a/daemon/tar.c
+++ b/daemon/tar.c
@@ -45,9 +45,9 @@ do_tar_in (const char *dir)
/* "tar -C /sysroot%s -xf -" but we have to quote the dir. */
if (asprintf_nowarn (&cmd, "tar -C %R -xf -", dir) == -1) {
err = errno;
- cancel_receive ();
+ r = cancel_receive ();
errno = err;
- reply_with_perror ("asprintf");
+ if (r != -2) reply_with_perror ("asprintf");
return -1;
}
@@ -57,9 +57,9 @@ do_tar_in (const char *dir)
fp = popen (cmd, "w");
if (fp == NULL) {
err = errno;
- cancel_receive ();
+ r = cancel_receive ();
errno = err;
- reply_with_perror ("%s", cmd);
+ if (r != -2) reply_with_perror ("%s", cmd);
free (cmd);
return -1;
}
@@ -72,8 +72,8 @@ do_tar_in (const char *dir)
r = receive_file (write_cb, &fd);
if (r == -1) { /* write error */
- cancel_receive ();
- reply_with_error ("write error on directory: %s", dir);
+ if (cancel_receive () != -2)
+ reply_with_error ("write error on directory: %s", dir);
pclose (fp);
return -1;
}
@@ -85,8 +85,9 @@ do_tar_in (const char *dir)
if (pclose (fp) != 0) {
if (r == -1) /* if r == 0, file transfer ended already */
- cancel_receive ();
- reply_with_error ("tar subcommand failed on directory: %s", dir);
+ r = cancel_receive ();
+ if (r != -2)
+ reply_with_error ("tar subcommand failed on directory: %s", dir);
return -1;
}
@@ -162,9 +163,9 @@ do_tXz_in (const char *dir, char filter)
/* "tar -C /sysroot%s -zxf -" but we have to quote the dir. */
if (asprintf_nowarn (&cmd, "tar -C %R -%cxf -", dir, filter) == -1) {
err = errno;
- cancel_receive ();
+ r = cancel_receive ();
errno = err;
- reply_with_perror ("asprintf");
+ if (r != -2) reply_with_perror ("asprintf");
return -1;
}
@@ -174,9 +175,9 @@ do_tXz_in (const char *dir, char filter)
fp = popen (cmd, "w");
if (fp == NULL) {
err = errno;
- cancel_receive ();
+ r = cancel_receive ();
errno = err;
- reply_with_perror ("%s", cmd);
+ if (r != -2) reply_with_perror ("%s", cmd);
free (cmd);
return -1;
}
@@ -186,8 +187,8 @@ do_tXz_in (const char *dir, char filter)
r = receive_file (write_cb, &fd);
if (r == -1) { /* write error */
- cancel_receive ();
- reply_with_error ("write error on directory: %s", dir);
+ r = cancel_receive ();
+ if (r != -2) reply_with_error ("write error on directory: %s", dir);
pclose (fp);
return -1;
}
@@ -199,8 +200,9 @@ do_tXz_in (const char *dir, char filter)
if (pclose (fp) != 0) {
if (r == -1) /* if r == 0, file transfer ended already */
- cancel_receive ();
- reply_with_error ("tar subcommand failed on directory: %s", dir);
+ r = cancel_receive ();
+ if (r != -2)
+ reply_with_error ("tar subcommand failed on directory: %s", dir);
return -1;
}
diff --git a/daemon/upload.c b/daemon/upload.c
index d93a5ade..9a6c8731 100644
--- a/daemon/upload.c
+++ b/daemon/upload.c
@@ -47,18 +47,18 @@ do_upload (const char *filename)
if (!is_dev) CHROOT_OUT;
if (fd == -1) {
err = errno;
- cancel_receive ();
+ r = cancel_receive ();
errno = err;
- reply_with_perror ("%s", filename);
+ if (r != -2) reply_with_perror ("%s", filename);
return -1;
}
r = receive_file (write_cb, &fd);
if (r == -1) { /* write error */
err = errno;
- cancel_receive ();
+ r = cancel_receive ();
errno = err;
- reply_with_error ("write error: %s", filename);
+ if (r != -2) reply_with_error ("write error: %s", filename);
close (fd);
return -1;
}
@@ -71,9 +71,10 @@ do_upload (const char *filename)
if (close (fd) == -1) {
err = errno;
if (r == -1) /* if r == 0, file transfer ended already */
- cancel_receive ();
+ r = cancel_receive ();
errno = err;
- reply_with_perror ("close: %s", filename);
+ if (r != -2)
+ reply_with_perror ("close: %s", filename);
return -1;
}
diff --git a/regressions/rhbz576879.sh b/regressions/rhbz576879.sh
index 7453ac78..d5711fd5 100755
--- a/regressions/rhbz576879.sh
+++ b/regressions/rhbz576879.sh
@@ -28,4 +28,14 @@ rm -f test1.img
-upload $srcdir/rhbz576879.sh /test.sh
# Shouldn't lose synchronization, so next command should work:
ping-daemon
-EOF \ No newline at end of file
+EOF
+
+# Second patch tests the problem found in comment 5 where both ends
+# send cancel messages simultaneously.
+
+../fish/guestfish -N disk <<EOF
+-tar-in /tmp/nosuchfile /blah
+ping-daemon
+EOF
+
+rm -f test1.img \ No newline at end of file
diff --git a/src/generator.ml b/src/generator.ml
index a995b4c9..73ab8fa1 100755
--- a/src/generator.ml
+++ b/src/generator.ml
@@ -6198,8 +6198,8 @@ and generate_daemon_actions () =
pr "\n";
pr " if (!xdr_guestfs_%s_args (xdr_in, &args)) {\n" name;
if is_filein then
- pr " cancel_receive ();\n";
- pr " reply_with_error (\"daemon failed to decode procedure arguments\");\n";
+ pr " if (cancel_receive () != -2)\n";
+ pr " reply_with_error (\"daemon failed to decode procedure arguments\");\n";
pr " goto done;\n";
pr " }\n";
let pr_args n =
@@ -6210,8 +6210,8 @@ and generate_daemon_actions () =
pr " sizeof (char *) * (args.%s.%s_len+1));\n" n n;
pr " if (%s == NULL) {\n" n;
if is_filein then
- pr " cancel_receive ();\n";
- pr " reply_with_perror (\"realloc\");\n";
+ pr " if (cancel_receive () != -2)\n";
+ pr " reply_with_perror (\"realloc\");\n";
pr " goto done;\n";
pr " }\n";
pr " %s[args.%s.%s_len] = NULL;\n" n n n;
@@ -6222,15 +6222,15 @@ and generate_daemon_actions () =
| Pathname n ->
pr_args n;
pr " ABS_PATH (%s, %s, goto done);\n"
- n (if is_filein then "cancel_receive ()" else "");
+ n (if is_filein then "cancel_receive ()" else "0");
| Device n ->
pr_args n;
pr " RESOLVE_DEVICE (%s, %s, goto done);\n"
- n (if is_filein then "cancel_receive ()" else "");
+ n (if is_filein then "cancel_receive ()" else "0");
| Dev_or_Path n ->
pr_args n;
pr " REQUIRE_ROOT_OR_RESOLVE_DEVICE (%s, %s, goto done);\n"
- n (if is_filein then "cancel_receive ()" else "");
+ n (if is_filein then "cancel_receive ()" else "0");
| String n -> pr_args n
| OptString n -> pr " %s = args.%s ? *args.%s : NULL;\n" n n n
| StringList n ->
@@ -6241,7 +6241,7 @@ and generate_daemon_actions () =
pr " * and perform device name translation. */\n";
pr " { int pvi; for (pvi = 0; physvols[pvi] != NULL; ++pvi)\n";
pr " RESOLVE_DEVICE (physvols[pvi], %s, goto done);\n"
- (if is_filein then "cancel_receive ()" else "");
+ (if is_filein then "cancel_receive ()" else "0");
pr " }\n";
| Bool n -> pr " %s = args.%s;\n" n n
| Int n -> pr " %s = args.%s;\n" n n
@@ -6257,7 +6257,7 @@ and generate_daemon_actions () =
(* Emit NEED_ROOT just once, even when there are two or
more Pathname args *)
pr " NEED_ROOT (%s, goto done);\n"
- (if is_filein then "cancel_receive ()" else "");
+ (if is_filein then "cancel_receive ()" else "0");
);
(* Don't want to call the impl with any FileIn or FileOut
generated by cgit (git 2.34.1) at 2025-05-17 18:38:59 +0000