search for: chroot_in

...--r--r-- 1 root root 0 Aug 18 10:37 /sysroot/../proc/modules ><fs> cat /../proc/modules libguestfs: error: open: /../proc/modules: No such file or directory The underlying reason for this seems to be that ll uses sysroot_path to establish a path before operating on it, whereas cat uses CHROOT_IN and CHROOT_OUT to open() inside a chroot. It seems to me that wherever a path is used directly by a command, there should never be a reason to use chroot because the real path can always be worked out. The only place I see for using a chroot is in command and sh. Is it worth making a bulk CHRO...

...> > return buf.st_size; > > > > } > > > > + > > +int > > +do_copy_attributes (const char *src, const char *dest, int > > permissions, int xattributes) +{ > > + int r; > > + struct stat srcstat, deststat; > > + > > + CHROOT_IN; > > + r = stat (src, &srcstat); > > + CHROOT_OUT; > > + > > + if (r == -1) { > > + reply_with_perror ("stat: %s", src); > > + return -1; > > + } > > + > > + CHROOT_IN; > > + r = stat (dest, &deststat); > &...

...s_int_xattr_list_val = calloc (r->guestfs_int_xattr_list_len, sizeof (guestfs_int_xattr)); @@ -134,34 +163,34 @@ getxattrs (const char *path, goto error; } - for (i = 0, j = 0; i < (size_t) len; i += strlen (&buf[i]) + 1, ++j) { + for (i = 0; names[i] != NULL; ++i) { CHROOT_IN; - vlen = getxattr (path, &buf[i], NULL, 0); + vlen = getxattr (path, names[i], NULL, 0); CHROOT_OUT; if (vlen == -1) { - reply_with_perror ("getxattr"); + reply_with_perror ("%s: getxattr", names[i]); goto error; } if (vlen >...

...s_int_xattr_list_val = calloc (r->guestfs_int_xattr_list_len, sizeof (guestfs_int_xattr)); @@ -134,34 +160,34 @@ getxattrs (const char *path, goto error; } - for (i = 0, j = 0; i < (size_t) len; i += strlen (&buf[i]) + 1, ++j) { + for (i = 0; names[i] != NULL; ++i) { CHROOT_IN; - vlen = getxattr (path, &buf[i], NULL, 0); + vlen = getxattr (path, names[i], NULL, 0); CHROOT_OUT; if (vlen == -1) { - reply_with_perror ("getxattr"); + reply_with_perror ("getxattr: %s", names[i]); goto error; } if (vlen >...

Hi, attached there is a prototype of patch for adding a new copy-attributes command. Such command would allow copy the attributes of a "file" to another, so for example in guestfish: copy-attributes foo bar permissions:true xattributes:false would only copy the permissions of foo to bar, not copying its extended attributes too. Just few notes: - my first daemon command, so

...at, zcat); > @@ -584,3 +585,46 @@ do_filesize (const char *path) > > return buf.st_size; > } > + > +int > +do_copy_attributes (const char *src, const char *dest, int permissions, int xattributes) > +{ > + int r; > + struct stat srcstat, deststat; > + > + CHROOT_IN; > + r = stat (src, &srcstat); > + CHROOT_OUT; > + > + if (r == -1) { > + reply_with_perror ("stat: %s", src); > + return -1; > + } > + > + CHROOT_IN; > + r = stat (dest, &deststat); > + CHROOT_OUT; > + > + if (r == -1) { > +...

On Tue, Dec 01, 2015 at 03:59:56PM +0100, Mateusz Guzik wrote: > CHROOT_IN/OUT around commandvf are definitely problematic. chroot should be > done in the child, which also removes the need to chroot out in the > parent. The CHROOT_IN/OUT business does need to be rewritten. Every instance where we currently do something like: CHROOT_IN; r = stat (fd, &sta...

...+ reply_with_perror ("/dev/null"); + return NULL; + } + if (bind_mount (&bind_state) == -1) return NULL; if (enable_network) { @@ -266,8 +279,10 @@ do_command (char *const *argv) return NULL; } + flags = COMMAND_FLAG_CHROOT_COPY_FILE_TO_STDIN | fd; + CHROOT_IN; - r = commandv (&out, &err, (const char * const *) argv); + r = commandvf (&out, &err, flags, (const char * const *) argv); CHROOT_OUT; free_bind_state (&bind_state); -- 2.1.0

...); +static char *_listxattrs (const char *path, ssize_t (*listxattr) (const char *path, char *list, size_t size), ssize_t *size); guestfs_int_xattr_list * do_getxattrs (const char *path) @@ -111,27 +112,10 @@ getxattrs (const char *path, size_t i, j; guestfs_int_xattr_list *r = NULL; - CHROOT_IN; - len = listxattr (path, NULL, 0); - CHROOT_OUT; - if (len == -1) { - reply_with_perror ("listxattr: %s", path); + buf = _listxattrs (path, listxattr, &len); + if (buf == NULL) + /* _listxattrs issues reply_with_perror already. */ goto error; - } - - buf = malloc (l...

...if (!(optargs_bitmask & GUESTFS_COPY_ATTRIBUTES_MODE_BITMASK)) + mode = 1; + if (!(optargs_bitmask & GUESTFS_COPY_ATTRIBUTES_XATTRIBUTES_BITMASK)) + xattributes = 1; + if (!(optargs_bitmask & GUESTFS_COPY_ATTRIBUTES_OWNERSHIP_BITMASK)) + ownership = 1; + } + + CHROOT_IN; + r = stat (src, &srcstat); + CHROOT_OUT; + + if (r == -1) { + reply_with_perror ("stat: %s", src); + return -1; + } + + CHROOT_IN; + r = stat (dest, &deststat); + CHROOT_OUT; + + if (r == -1) { + reply_with_perror ("stat: %s", dest); + return -1; +...

...is command is not meant for - * serious use anyway, just for quick interactive sessions. - */ - char * do_ll (const char *path) { int r; char *out; CLEANUP_FREE char *err = NULL; + CLEANUP_FREE char *rpath = NULL; CLEANUP_FREE char *spath = NULL; - spath = sysroot_path (path); + CHROOT_IN; + rpath = realpath (path, NULL); + CHROOT_OUT; + if (rpath == NULL) { + reply_with_perror ("%s", path); + return NULL; + } + + spath = sysroot_path (rpath); if (!spath) { reply_with_perror ("malloc"); return NULL; @@ -131,9 +134,18 @@ do_llz (const char...

This started as a simple warning-elimination change. I'll get back to that series shortly ;-) It turned into a factorization and constification exercise during which I got a taste of ocaml. Thanks to Rich Jones for help with a few snippets in generator.ml. The overall result is that many previously-manually-maintained bits from daemon/*.c functions are now hoisted into the automatically-

...size_t size), + size_t *size_r) +{ ssize_t r; char *buf; size_t len; @@ -496,49 +506,7 @@ do_getxattr (const char *path, const char *name, size_t *size_r) char * do_lgetxattr (const char *path, const char *name, size_t *size_r) { - ssize_t r; - char *buf; - size_t len; - - CHROOT_IN; - r = lgetxattr (path, name, NULL, 0); - CHROOT_OUT; - if (r == -1) { - reply_with_perror ("lgetxattr"); - return NULL; - } - - len = r; - - if (len > XATTR_SIZE_MAX) { - reply_with_error ("extended attribute is too large"); - return NULL; - } - - buf = m...

v1 here: https://www.redhat.com/archives/libguestfs/2020-March/msg00099.html This one fixes most of the points picked up in review, and does not strdup the strings which should keep down memory usage if that is a concern. Rich.

On Tue, Dec 01, 2015 at 04:58:11PM +0000, Richard W.M. Jones wrote: > On Tue, Dec 01, 2015 at 03:59:56PM +0100, Mateusz Guzik wrote: > > CHROOT_IN/OUT around commandvf are definitely problematic. chroot should be > > done in the child, which also removes the need to chroot out in the > > parent. > > The CHROOT_IN/OUT business does need to be rewritten. Every > instance where we currently do something like: > >...

- add a flag to request chroot for the process, which is done only as very last (before chdir) operation before exec'ing the process in the child: this avoids using CHROOT_IN & CHROOT_OUT around command* invocations, and reduces the code spent in chroot mode - add failure checks for dup2 and open done in child, not proceeding to executing the process if they fail - open /dev/null without O_CLOEXEC, so it stays available for the exec'ed process, and thus we...

...t ?name () = Chroot.create (Sysroot.sysroot ()) ?name this way it can be used like: let chroot = create_chroot ~name:(sprintf "file: %s" path) in > + > + let statbuf = Chroot.f chroot lstat path in Hm is chroot needed for this? The current C implementation does not use CHROOT_IN/OUT, and it does not even resolve symlinks, so it should be safe. -- Pino Toscano

https://bugzilla.redhat.com/show_bug.cgi?id=1811539 Commands including virt-diff which read extended attributes will sometimes fail on NTFS filesystems that are using system compressed. The reason is complex, see comment 5 of the bug linked above. This patch filters out the troublesome xattr. For justification, see the comment I added in patch 3. Patch 1 & 2 are refactoring. I was on the

...return NULL; nit: this leaks the fd on error, but it may not matter much. > if (enable_network) { > @@ -266,8 +279,10 @@ do_command (char *const *argv) > return NULL; > } > nit: same. > + flags = COMMAND_FLAG_CHROOT_COPY_FILE_TO_STDIN | fd; > + > CHROOT_IN; > - r = commandv (&out, &err, (const char * const *) argv); > + r = commandvf (&out, &err, flags, (const char * const *) argv); > CHROOT_OUT; > > free_bind_state (&bind_state); According to the analysis in https://bugzilla.redhat.com/show_bug.cgi?id=12...

...const char *path) } char * -do_mkdtemp (char *template) +do_mkdtemp (const char *template) { - char *r; - - NEED_ROOT (return NULL); - ABS_PATH (template, return NULL); + char *r = strdup (template); + if (r == NULL) { + reply_with_perror ("strdup"); + return NULL; + } CHROOT_IN; - r = mkdtemp (template); + r = mkdtemp (r); CHROOT_OUT; - if (r == NULL) { + if (r == NULL) reply_with_perror ("mkdtemp: %s", template); - return NULL; - } - /* The caller will free template AND try to free the return value, - * so we must make a copy here. - */ -...