[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 07/31] 9pfs: introduce relative_openat_nofollow()
From: |
Greg Kurz |
Subject: |
Re: [Qemu-devel] [PULL 07/31] 9pfs: introduce relative_openat_nofollow() helper |
Date: |
Tue, 28 Feb 2017 01:33:55 +0100 |
On Mon, 27 Feb 2017 17:37:56 -0600
Eric Blake <address@hidden> wrote:
> On 02/27/2017 04:59 PM, Greg Kurz wrote:
> > When using the passthrough security mode, symbolic links created by the
> > guest are actual symbolic links on the host file system.
> >
>
> Hmm, I just barely started reviewing the series, and see a pull request.
> At this point, anything I point out can probably be done as followup
> patches rather than forcing a respin of the pull (and soft freeze is
> appropriate for that).
>
Yes but I now realize I have another nit... Patch 2/31 should have
From: Pradeep <address@hidden>
but for unknown reasons, it got dropped at some point, and cannot be
fixed in a followup patch.
For simplicity, I guess I'd rather fix all the issues and respin a new
pull tomorrow.
> > Suggested-by: Jann Horn <address@hidden>
> > Signed-off-by: Greg Kurz <address@hidden>
> > Reviewed-by: Stefan Hajnoczi <address@hidden>
> > (renamed openat_nofollow() to relative_openat_nofollow(),
> > assert path is relative, Greg Kurz)
> > Signed-off-by: Greg Kurz <address@hidden>
> > ---
>
> > +int relative_openat_nofollow(int dirfd, const char *path, int flags,
> > + mode_t mode)
> > +{
> > + int fd;
> > +
> > + assert(path[0] != '/');
>
> If you move this assert...
>
> > +
> > + fd = dup(dirfd);
> > + if (fd == -1) {
> > + return -1;
> > + }
> > +
> > + while (*path) {
> > + const char *c;
> > + int next_fd;
> > + char *head;
>
> ...here, you can make sure there are no 'a//b' issues to worry about.
>
> > +
> > + head = g_strdup(path);
> > + c = strchr(path, '/');
> > + if (c) {
> > + head[c - path] = 0;
> > + next_fd = openat_dir(fd, head);
> > + } else {
> > + next_fd = openat_file(fd, head, flags, mode);
> > + }
> > + g_free(head);
> > + if (next_fd == -1) {
> > + close_preserve_errno(fd);
> > + return -1;
> > + }
> > + close(fd);
> > + fd = next_fd;
> > +
> > + if (!c) {
> > + break;
> > + }
> > + path = c + 1;
>
> or else add an assert here.
>
>
> > +static inline int openat_file(int dirfd, const char *name, int flags,
> > + mode_t mode)
> > +{
> > + int fd, serrno;
> > +
> > + fd = openat(dirfd, name, flags | O_NOFOLLOW | O_NOCTTY | O_NONBLOCK,
> > + mode);
> > + if (fd == -1) {
> > + return -1;
> > + }
> > +
> > + serrno = errno;
> > + /* O_NONBLOCK was only needed to open the file. Let's drop it. */
> > + assert(!fcntl(fd, F_SETFL, flags));
>
> Ewww. Side effect inside an assert(). :(
>
pgpX6e9ZhpXPX.pgp
Description: OpenPGP digital signature
- [Qemu-devel] [PULL 00/31] 9p patches 2017-02-27 for 2.9 soft freeze, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 05/31] 9pfs: remove side-effects in local_init(), Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 07/31] 9pfs: introduce relative_openat_nofollow() helper, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 08/31] 9pfs: local: keep a file descriptor on the shared folder, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 11/31] 9pfs: local: llistxattr: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 13/31] 9pfs: local: lremovexattr: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 06/31] 9pfs: remove side-effects in local_open() and local_opendir(), Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 10/31] 9pfs: local: lgetxattr: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 01/31] 9pfs: fix v9fs_lock error case, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 02/31] fsdev: add IO throttle support to fsdev devices, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 12/31] 9pfs: local: lsetxattr: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 15/31] 9pfs: local: remove: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 03/31] throttle: factor out duplicate code, Greg Kurz, 2017/02/27