mirrors/libguestfs
1
0
Fork 0
mirror of https://github.com/libguestfs/libguestfs.git synced 2026-03-22 07:03:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

daemon: Remove remaining uses of custom printf %Q and %R

Browse Source 
We have traditionally used custom printf formatters %Q and %R, where
%Q replaces the argument with a shell-quoted string, and %R replaces
the argument with a sysroot-prefixed shell-quoted string.  They are
actually pretty useful, but unfortunately only supported by glibc.

We only used them in about a dozen places in the daemon (much code
having been replaced by OCaml which does not need them).

In every remaining case we were constructing a command using code like
this:

  asprintf_nowarn (&cmd,
         "cd %Q && find -print0 | %s -0 -o -H %s --quiet", ...);

We can replace this with:

  char *cmd;
  size_t cmd_size;
  fp = open_memstream (&cmd, &cmd_size);
  fprintf (fp, "cd ");
  shell_quote (dir, fp);
  fprintf (fp, " && find -print0 | %s -0 -o -H %s --quiet", ...);
  fclose (fp);

(cherry picked from commit 0b3c6cc0c0)
This commit is contained in:
Richard W.M. Jones
2022-08-16 09:04:33 +01:00
parent 4b83ed8a2d
commit 8bf4455936
17 changed files with 166 additions and 228 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
daemon/Makefile.am
View File

@@ -126,7 +126,6 @@ guestfsd_SOURCES = \
file.c \
fill.c \
find.c \
format.c \
fs-min-size.c \
fsck.c \
fstrim.c \

22
daemon/base64.c
View File

@@ -44,15 +44,22 @@ do_base64_in (const char *file)
int err, r;
FILE *fp;
CLEANUP_FREE char *cmd = NULL;
size_t cmd_size;
int fd;
if (asprintf_nowarn (&cmd, "%s -d -i > %R", "base64", file) == -1) {
fp = open_memstream (&cmd, &cmd_size);
if (fp == NULL) {
cmd_error:
err = errno;
cancel_receive ();
errno = err;
reply_with_perror ("asprintf");
reply_with_perror ("malloc");
return -1;
}
fprintf (fp, "%s -d -i > ", "base64");
sysroot_shell_quote (file, fp);
if (fclose (fp) == EOF)
goto cmd_error;
if (verbose)
fprintf (stderr, "%s\n", cmd);
@@ -104,6 +111,7 @@ do_base64_out (const char *file)
int r;
FILE *fp;
CLEANUP_FREE char *cmd = NULL;
size_t cmd_size;
CLEANUP_FREE char *buffer = NULL;
buffer = malloc (GUESTFS_MAX_CHUNK_SIZE);
@@ -130,10 +138,16 @@ do_base64_out (const char *file)
}
/* Construct the command. */
if (asprintf_nowarn (&cmd, "%s %Q", "base64", buf) == -1) {
reply_with_perror ("asprintf");
fp = open_memstream (&cmd, &cmd_size);
if (fp == NULL) {
cmd_error:
reply_with_perror ("open_memstream");
return -1;
}
fprintf (fp, "base64 ");
shell_quote (buf, fp);
if (fclose (fp) == EOF)
goto cmd_error;
if (verbose)
fprintf (stderr, "%s\n", cmd);

15
daemon/checksum.c
View File

@@ -126,6 +126,7 @@ do_checksums_out (const char *csumtype, const char *dir)
CLEANUP_FREE char *str = NULL;
CLEANUP_FREE char *sysrootdir = NULL;
CLEANUP_FREE char *cmd = NULL;
size_t cmd_size;
FILE *fp;
str = malloc (GUESTFS_MAX_CHUNK_SIZE);
@@ -154,12 +155,18 @@ do_checksums_out (const char *csumtype, const char *dir)
return -1;
}
cmd = NULL;
if (asprintf_nowarn (&cmd, "cd %Q && %s -type f -print0 | %s -0 %s",
sysrootdir, "find", "xargs", program) == -1) {
reply_with_perror ("asprintf");
fp = open_memstream (&cmd, &cmd_size);
if (fp == NULL) {
cmd_error:
reply_with_perror ("open_memstream");
return -1;
}
fprintf (fp, "cd ");
shell_quote (sysrootdir, fp);
fprintf (fp, " && %s -type f -print0 | %s -0 %s",
"find", "xargs", program);
if (fclose (fp) == EOF)
goto cmd_error;
if (verbose)
fprintf (stderr, "%s\n", cmd);

25
daemon/compress.c
View File

@@ -34,6 +34,7 @@ do_compressX_out (const char *file, const char *filter, int is_device)
int r;
FILE *fp;
CLEANUP_FREE char *cmd = NULL;
size_t cmd_size;
CLEANUP_FREE char *buf = NULL;
buf = malloc (GUESTFS_MAX_CHUNK_SIZE);
@@ -53,17 +54,21 @@ do_compressX_out (const char *file, const char *filter, int is_device)
* unhelpfully refuses to compress anything that isn't a regular
* file.
*/
if (!is_device) {
if (asprintf_nowarn (&cmd, "%s %R", filter, file) == -1) {
reply_with_perror ("asprintf");
return -1;
}
} else {
if (asprintf_nowarn (&cmd, "%s < %Q", filter, file) == -1) {
reply_with_perror ("asprintf");
return -1;
}
fp = open_memstream (&cmd, &cmd_size);
if (fp == NULL) {
cmd_error:
reply_with_perror ("open_memstream");
return -1;
}
fprintf (fp, "%s ", filter);
if (!is_device) {
sysroot_shell_quote (file, fp);
} else {
fprintf (fp, "< ");
shell_quote (file, fp);
}
if (fclose (fp) == EOF)
goto cmd_error;
if (verbose)
fprintf (stderr, "%s\n", cmd);

15
daemon/cpio.c
View File

@@ -41,6 +41,7 @@ do_cpio_out (const char *dir, const char *format)
int r;
FILE *fp;
CLEANUP_FREE char *cmd = NULL;
size_t cmd_size;
CLEANUP_FREE char *buffer = NULL;
buffer = malloc (GUESTFS_MAX_CHUNK_SIZE);
@@ -76,13 +77,17 @@ do_cpio_out (const char *dir, const char *format)
else
format = "newc";
if (asprintf_nowarn (&cmd, "cd %Q && find -print0 | %s -0 -o -H %s --quiet",
buf,
"cpio",
format) == -1) {
reply_with_perror ("asprintf");
fp = open_memstream (&cmd, &cmd_size);
if (fp == NULL) {
cmd_error:
reply_with_perror ("open_memstream");
return -1;
}
fprintf (fp, "cd ");
shell_quote (buf, fp);
fprintf (fp, " && find -print0 | %s -0 -o -H %s --quiet", "cpio", format);
if (fclose (fp) == EOF)
goto cmd_error;
if (verbose)
fprintf (stderr, "%s\n", cmd);

3
daemon/daemon.h
View File

@@ -60,8 +60,10 @@ extern const char *sysroot;
extern size_t sysroot_len;
extern dev_t root_device;
extern void shell_quote (const char *str, FILE *fp);
extern char *sysroot_path (const char *path);
extern char *sysroot_realpath (const char *path);
extern void sysroot_shell_quote (const char *path, FILE *fp);
extern int is_root_device (const char *device);
extern int is_device_parameter (const char *device);
extern int xwrite (int sock, const void *buf, size_t len)
@@ -85,7 +87,6 @@ extern void udev_settle (void);
extern int random_name (char *template);
extern char *get_random_uuid (void);
extern char *make_exclude_from_file (const char *function, char *const *excludes);
extern int asprintf_nowarn (char **strp, const char *fmt, ...);
extern char *read_whole_file (const char *filename, size_t *size_r);
/* mountable functions (in utils.c) */

16
daemon/ext2.c
View File

@@ -1314,12 +1314,24 @@ int
do_mklost_and_found (const char *mountpoint)
{
CLEANUP_FREE char *cmd = NULL;
size_t cmd_size;
FILE *fp;
int r;
if (asprintf_nowarn (&cmd, "cd %R && mklost+found", mountpoint) == -1) {
reply_with_perror ("asprintf");
fp = open_memstream (&cmd, &cmd_size);
if (fp == NULL) {
cmd_error:
reply_with_perror ("open_memstream");
return -1;
}
fprintf (fp, "cd ");
sysroot_shell_quote (mountpoint, fp);
fprintf (fp, " && mklost+found");
if (fclose (fp) == EOF)
goto cmd_error;
if (verbose)
fprintf (stderr, "%s\n", cmd);
r = system (cmd);
if (r == -1) {

12
daemon/file.c
View File

@@ -452,6 +452,7 @@ do_zfile (const char *method, const char *path)
size_t len;
const char *zcat;
CLEANUP_FREE char *cmd = NULL;
size_t cmd_size;
FILE *fp;
char line[256];
@@ -464,10 +465,17 @@ do_zfile (const char *method, const char *path)
return NULL;
}
if (asprintf_nowarn (&cmd, "%s %R | file -bsL -", zcat, path) == -1) {
reply_with_perror ("asprintf");
fp = open_memstream (&cmd, &cmd_size);
if (fp == NULL) {
cmd_error:
reply_with_perror ("open_memstream");
return NULL;
}
fprintf (fp, "%s ", zcat);
sysroot_shell_quote (path, fp);
fprintf (fp, " | file -bsL -");
if (fclose (fp) == EOF)
goto cmd_error;
if (verbose)
fprintf (stderr, "%s\n", cmd);

12
daemon/find.c
View File

@@ -57,6 +57,7 @@ do_find0 (const char *dir)
int r;
FILE *fp;
CLEANUP_FREE char *cmd = NULL;
size_t cmd_size;
CLEANUP_FREE char *sysrootdir = NULL;
size_t sysrootdirlen;
CLEANUP_FREE char *str = NULL;
@@ -85,10 +86,17 @@ do_find0 (const char *dir)
sysrootdirlen = strlen (sysrootdir);
if (asprintf_nowarn (&cmd, "find %Q -print0", sysrootdir) == -1) {
reply_with_perror ("asprintf");
fp = open_memstream (&cmd, &cmd_size);
if (fp == NULL) {
cmd_error:
reply_with_perror ("open_memstream");
return -1;
}
fprintf (fp, "find ");
shell_quote (sysrootdir, fp);
fprintf (fp, " -print0");
if (fclose (fp) == EOF)
goto cmd_error;
if (verbose)
fprintf (stderr, "%s\n", cmd);

49
daemon/format.c
View File

@@ -1,49 +0,0 @@
/* libguestfs - the guestfsd daemon
* Copyright (C) 2010-2020 Red Hat Inc.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/
#include <config.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "daemon.h"
/* This just stops gcc from giving a warning about our custom printf
* formatters %Q and %R. See guestfs-hacking(1) for more
* info about these. In GCC 4.8.0 the warning is even harder to
* 'trick', hence the need for the #pragma directives.
*/
#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >= 4.8.0 */
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wsuggest-attribute=format"
#endif
int
asprintf_nowarn (char **strp, const char *fmt, ...)
{
int r;
va_list args;
va_start (args, fmt);
r = vasprintf (strp, fmt, args);
va_end (args);
return r;
}
#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >= 4.8.0 */
#pragma GCC diagnostic pop
#endif

86
daemon/guestfsd.c
View File

@@ -53,17 +53,6 @@
#include "daemon.h"
static void makeraw (const char *channel, int fd);
static int print_shell_quote (FILE *stream, const struct printf_info *info, const void *const *args);
static int print_sysroot_shell_quote (FILE *stream, const struct printf_info *info, const void *const *args);
#ifdef HAVE_REGISTER_PRINTF_SPECIFIER
static int print_arginfo (const struct printf_info *info, size_t n, int *argtypes, int *size);
#else
#ifdef HAVE_REGISTER_PRINTF_FUNCTION
static int print_arginfo (const struct printf_info *info, size_t n, int *argtypes);
#else
#error "HAVE_REGISTER_PRINTF_{SPECIFIER|FUNCTION} not defined"
#endif
#endif
#ifdef WIN32
static int
@@ -115,19 +104,6 @@ main (int argc, char *argv[])
if (winsock_init () == -1)
error (EXIT_FAILURE, 0, "winsock initialization failed");
#ifdef HAVE_REGISTER_PRINTF_SPECIFIER
/* http://udrepper.livejournal.com/20948.html */
register_printf_specifier ('Q', print_shell_quote, print_arginfo);
register_printf_specifier ('R', print_sysroot_shell_quote, print_arginfo);
#else
#ifdef HAVE_REGISTER_PRINTF_FUNCTION
register_printf_function ('Q', print_shell_quote, print_arginfo);
register_printf_function ('R', print_sysroot_shell_quote, print_arginfo);
#else
#error "HAVE_REGISTER_PRINTF_{SPECIFIER|FUNCTION} not defined"
#endif
#endif
/* XXX The appliance /init script sets LD_PRELOAD=../libSegFault.so.
* However if we CHROOT_IN to the sysroot that file might not exist,
* resulting in all commands failing. What we'd really like to do
@@ -347,62 +323,30 @@ makeraw (const char *channel, int fd)
}
/**
* printf helper function so we can use C<%Q> ("quoted") and C<%R> to
* print shell-quoted strings. See L<guestfs-hacking(1)> for more
* details.
* Write C<str> to the file C<fp>, ensuring it is shell quoted.
*/
static int
print_shell_quote (FILE *stream,
const struct printf_info *info ATTRIBUTE_UNUSED,
const void *const *args)
void
shell_quote (const char *str, FILE *fp)
{
#define SAFE(c) (c_isalnum((c)) || \
(c) == '/' || (c) == '-' || (c) == '_' || (c) == '.')
int i, len;
const char *str = *((const char **) (args[0]));
size_t i;
for (i = len = 0; str[i]; ++i) {
for (i = 0; str[i]; ++i) {
if (!SAFE (str[i])) {
putc ('\\', stream);
len ++;
putc ('\\', fp);
}
putc (str[i], stream);
len ++;
putc (str[i], fp);
}
return len;
}
static int
print_sysroot_shell_quote (FILE *stream,
const struct printf_info *info,
const void *const *args)
/**
* Write C<sysroot> + C<path> to the file C<fp>, ensuring it is shell
* quoted. The path must be an absolute path.
*/
void
sysroot_shell_quote (const char *path, FILE *fp)
{
fputs (sysroot, stream);
return sysroot_len + print_shell_quote (stream, info, args);
fputs (sysroot, fp);
shell_quote (path, fp);
}
#ifdef HAVE_REGISTER_PRINTF_SPECIFIER
static int
print_arginfo (const struct printf_info *info ATTRIBUTE_UNUSED,
size_t n, int *argtypes, int *size)
{
if (n > 0) {
argtypes[0] = PA_STRING;
size[0] = sizeof (const char *);
}
return 1;
}
#else
#ifdef HAVE_REGISTER_PRINTF_FUNCTION
static int
print_arginfo (const struct printf_info *info, size_t n, int *argtypes)
{
if (n > 0)
argtypes[0] = PA_STRING;
return 1;
}
#else
#error "HAVE_REGISTER_PRINTF_{SPECIFIER|FUNCTION} not defined"
#endif
#endif

29
daemon/initrd.c
View File

@@ -36,6 +36,7 @@ do_initrd_list (const char *path)
{
FILE *fp;
CLEANUP_FREE char *cmd = NULL;
size_t cmd_size;
CLEANUP_FREE_STRINGSBUF DECLARE_STRINGSBUF (filenames);
CLEANUP_FREE char *filename = NULL;
size_t allocsize;
@@ -43,10 +44,17 @@ do_initrd_list (const char *path)
int ret;
/* "zcat /sysroot/<path> | cpio --quiet -it", but path must be quoted. */
if (asprintf_nowarn (&cmd, "zcat %R | cpio --quiet -it", path) == -1) {
reply_with_perror ("asprintf");
fp = open_memstream (&cmd, &cmd_size);
if (fp == NULL) {
cmd_error:
reply_with_perror ("open_memstream");
return NULL;
}
fprintf (fp, "zcat ");
sysroot_shell_quote (path, fp);
fprintf (fp, " | cpio --quiet -it");
if (fclose (fp) == EOF)
goto cmd_error;
if (verbose)
fprintf (stderr, "%s\n", cmd);
@@ -92,6 +100,8 @@ do_initrd_cat (const char *path, const char *filename, size_t *size_r)
{
char tmpdir[] = "/tmp/initrd-cat-XXXXXX";
CLEANUP_FREE char *cmd = NULL;
size_t cmd_size;
FILE *fp;
struct stat statbuf;
int fd, r;
char *ret = NULL;
@@ -108,12 +118,21 @@ do_initrd_cat (const char *path, const char *filename, size_t *size_r)
* cpio is silent in this case.
*/
/* "zcat /sysroot/<path> | cpio --quiet -id file", but paths must be quoted */
if (asprintf_nowarn (&cmd, "cd %Q && zcat %R | cpio --quiet -id %Q",
tmpdir, path, filename) == -1) {
reply_with_perror ("asprintf");
fp = open_memstream (&cmd, &cmd_size);
if (fp == NULL) {
cmd_error:
reply_with_perror ("open_memstream");
rmdir (tmpdir);
return NULL;
}
fprintf (fp, "cd ");
shell_quote (tmpdir, fp);
fprintf (fp, " && zcat ");
sysroot_shell_quote (path, fp);
fprintf (fp, " | cpio --quiet -id ");
shell_quote (filename, fp);
if (fclose (fp) == EOF)
goto cmd_error;
r = system (cmd);
if (r == -1) {

60
daemon/tar.c
View File

@@ -140,6 +140,7 @@ do_tar_in (const char *dir, const char *compress, int xattrs, int selinux, int a
int err, r;
FILE *fp;
CLEANUP_FREE char *cmd = NULL;
size_t cmd_size;
char error_file[] = "/tmp/tarXXXXXX";
int fd, chown_supported;
@@ -186,25 +187,31 @@ do_tar_in (const char *dir, const char *compress, int xattrs, int selinux, int a
close (fd);
/* "tar -C /sysroot%s -xf -" but we have to quote the dir. */
if (asprintf_nowarn (&cmd, "%s -C %R%s -xf - %s%s%s%s2> %s",
"tar",
dir, filter,
chown_supported ? "" : "--no-same-owner ",
/* --xattrs-include=* is a workaround for a bug
* in tar, and hopefully won't be required
* forever. See RHBZ#771927.
*/
xattrs ? "--xattrs --xattrs-include='*' " : "",
selinux ? "--selinux " : "",
acls ? "--acls " : "",
error_file) == -1) {
fp = open_memstream (&cmd, &cmd_size);
if (fp == NULL) {
cmd_error:
err = errno;
r = cancel_receive ();
errno = err;
reply_with_perror ("asprintf");
reply_with_perror ("open_memstream");
unlink (error_file);
return -1;
}
fprintf (fp, "tar -C ");
sysroot_shell_quote (dir, fp);
fprintf (fp, "%s -xf - %s%s%s%s2> %s",
filter,
chown_supported ? "" : "--no-same-owner ",
/* --xattrs-include=* is a workaround for a bug
* in tar, and hopefully won't be required
* forever. See RHBZ#771927.
*/
xattrs ? "--xattrs --xattrs-include='*' " : "",
selinux ? "--selinux " : "",
acls ? "--acls " : "",
error_file);
if (fclose (fp) == EOF)
goto cmd_error;
if (verbose)
fprintf (stderr, "%s\n", cmd);
@@ -285,6 +292,7 @@ do_tar_out (const char *dir, const char *compress, int numericowner,
FILE *fp;
CLEANUP_UNLINK_FREE char *exclude_from_file = NULL;
CLEANUP_FREE char *cmd = NULL;
size_t cmd_size;
CLEANUP_FREE char *buffer = NULL;
buffer = malloc (GUESTFS_MAX_CHUNK_SIZE);
@@ -347,18 +355,24 @@ do_tar_out (const char *dir, const char *compress, int numericowner,
}
/* "tar -C /sysroot%s -cf - ." but we have to quote the dir. */
if (asprintf_nowarn (&cmd, "%s -C %Q%s%s%s%s%s%s%s -cf - .",
"tar",
buf, filter,
numericowner ? " --numeric-owner" : "",
exclude_from_file ? " -X " : "",
exclude_from_file ? exclude_from_file : "",
xattrs ? " --xattrs" : "",
selinux ? " --selinux" : "",
acls ? " --acls" : "") == -1) {
reply_with_perror ("asprintf");
fp = open_memstream (&cmd, &cmd_size);
if (fp == NULL) {
cmd_error:
reply_with_perror ("open_memstream");
return -1;
}
fprintf (fp, "tar -C ");
shell_quote (buf, fp);
fprintf (fp, "%s%s%s%s%s%s%s -cf - .",
filter,
numericowner ? " --numeric-owner" : "",
exclude_from_file ? " -X " : "",
exclude_from_file ? exclude_from_file : "",
xattrs ? " --xattrs" : "",
selinux ? " --selinux" : "",
acls ? " --acls" : "");
if (fclose (fp) == EOF)
goto cmd_error;
if (verbose)
fprintf (stderr, "%s\n", cmd);

1
docs/C_SOURCE_FILES
View File

@@ -86,7 +86,6 @@ daemon/fallocate.c
daemon/file.c
daemon/fill.c
daemon/find.c
daemon/format.c
daemon/fs-min-size.c
daemon/fsck.c
daemon/fstrim.c

31
docs/guestfs-hacking.pod
View File

@@ -824,37 +824,6 @@ and CC to L<rjones@redhat.com>.
You do not need to subscribe to the mailing list if you don’t want to.
There may be a short delay while your message is moderated.
=head2 DAEMON CUSTOM PRINTF FORMATTERS
In the daemon code we have created custom printf formatters C<%Q> and
C<%R>, which are used to do shell quoting.
=over 4
=item %Q
Simple shell quoted string. Any spaces or other shell characters are
escaped for you.
=item %R
Same as C<%Q> except the string is treated as a path which is prefixed
by the sysroot.
=back
For example:
asprintf (&cmd, "cat %R", path);
would produce C<cat /sysroot/some\ path\ with\ spaces>
I<Note:> Do I<not> use these when you are passing parameters to the
C<command{,r,v,rv}()> functions. These parameters do NOT need to be
quoted because they are not passed via the shell (instead, straight to
exec). You probably want to use the C<sysroot_path()> function
however.
=head2 INTERNATIONALIZATION (I18N) SUPPORT
We support i18n (gettext anyhow) in the library.

16
m4/guestfs-daemon.m4
View File

@@ -58,22 +58,6 @@ if test "x$enable_daemon" = "xyes"; then
fi
AC_MSG_RESULT([$DAEMON_SUPERMIN_DIR])
AC_SUBST([DAEMON_SUPERMIN_DIR])
dnl For modified printf in the daemon, we need glibc either (old-style)
dnl register_printf_function or (new-style) register_printf_specifier.
AC_CHECK_FUNC([register_printf_specifier],[
AC_DEFINE([HAVE_REGISTER_PRINTF_SPECIFIER],[1],
[Define to 1 if you have new-style register_printf_specifier.])
],[
AC_CHECK_FUNC([register_printf_function],[
AC_DEFINE([HAVE_REGISTER_PRINTF_FUNCTION],[1],
[Define to 1 if you have old-style register_printf_function.])
],[
AC_MSG_FAILURE(
[No support for glibc-style extended printf formatters.
This means you either have a very old glibc (pre-2.0) or you
are using some other libc where this is not supported.])])])
fi
AM_CONDITIONAL([INSTALL_DAEMON],[test "x$enable_install_daemon" = "xyes"])

1
po/POTFILES
View File

@@ -65,7 +65,6 @@ daemon/fallocate.c
daemon/file.c
daemon/fill.c
daemon/find.c
daemon/format.c
daemon/fs-min-size.c
daemon/fsck.c
daemon/fstrim.c