From: Jens Freimann <jfreimann@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: virtio-fs@redhat.com, qemu-devel@nongnu.org
Subject: Re: [Virtio-fs] [PATCH] virtiofsd: avoid /proc/self/fd tempdir
Date: Tue, 6 Oct 2020 15:43:38 +0200 [thread overview]
Message-ID: <20201006134338.GA49600@linux.fritz.box> (raw)
In-Reply-To: <20201006095826.59813-1-stefanha@redhat.com>
On Tue, Oct 06, 2020 at 10:58:26AM +0100, Stefan Hajnoczi wrote:
>In order to prevent /proc/self/fd escapes a temporary directory is
>created where /proc/self/fd is bind-mounted. This doesn't work on
>read-only file systems.
>
>Avoid the temporary directory by bind-mounting /proc/self/fd over /proc.
>This does not affect other processes since we remounted / with MS_REC |
>MS_SLAVE. /proc must exist and virtiofsd does not use it so it's safe to
>do this.
>
>Path traversal can be tested with the following function:
>
> static void test_proc_fd_escape(struct lo_data *lo)
> {
> int fd;
> int level = 0;
> ino_t last_ino = 0;
>
> fd = lo->proc_self_fd;
> for (;;) {
> struct stat st;
>
> if (fstat(fd, &st) != 0) {
> perror("fstat");
> return;
> }
> if (last_ino && st.st_ino == last_ino) {
> fprintf(stderr, "inode number unchanged, stopping\n");
> return;
> }
> last_ino = st.st_ino;
>
> fprintf(stderr, "Level %d dev %lu ino %lu\n", level,
> (unsigned long)st.st_dev,
> (unsigned long)last_ino);
> fd = openat(fd, "..", O_PATH | O_DIRECTORY | O_NOFOLLOW);
> level++;
> }
> }
>
>Before and after this patch only Level 0 is displayed. Without
>/proc/self/fd bind-mount protection it is possible to traverse parent
>directories.
>
>Fixes: 397ae982f4df4 ("virtiofsd: jail lo->proc_self_fd")
>Cc: Miklos Szeredi <mszeredi@redhat.com>
>Cc: Jens Freimann <jfreimann@redhat.com>
>Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Thanks Stefan, it fixes the problem we had!
Tested-by: Jens Freimann <jfreimann@redhat.com>
Reviewed-by: Jens Freimann <jfreimann@redhat.com>
regards,
Jens
WARNING: multiple messages have this Message-ID (diff)
From: Jens Freimann <jfreimann@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: virtio-fs@redhat.com, Miklos Szeredi <mszeredi@redhat.com>,
qemu-devel@nongnu.org,
"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [PATCH] virtiofsd: avoid /proc/self/fd tempdir
Date: Tue, 6 Oct 2020 15:43:38 +0200 [thread overview]
Message-ID: <20201006134338.GA49600@linux.fritz.box> (raw)
In-Reply-To: <20201006095826.59813-1-stefanha@redhat.com>
On Tue, Oct 06, 2020 at 10:58:26AM +0100, Stefan Hajnoczi wrote:
>In order to prevent /proc/self/fd escapes a temporary directory is
>created where /proc/self/fd is bind-mounted. This doesn't work on
>read-only file systems.
>
>Avoid the temporary directory by bind-mounting /proc/self/fd over /proc.
>This does not affect other processes since we remounted / with MS_REC |
>MS_SLAVE. /proc must exist and virtiofsd does not use it so it's safe to
>do this.
>
>Path traversal can be tested with the following function:
>
> static void test_proc_fd_escape(struct lo_data *lo)
> {
> int fd;
> int level = 0;
> ino_t last_ino = 0;
>
> fd = lo->proc_self_fd;
> for (;;) {
> struct stat st;
>
> if (fstat(fd, &st) != 0) {
> perror("fstat");
> return;
> }
> if (last_ino && st.st_ino == last_ino) {
> fprintf(stderr, "inode number unchanged, stopping\n");
> return;
> }
> last_ino = st.st_ino;
>
> fprintf(stderr, "Level %d dev %lu ino %lu\n", level,
> (unsigned long)st.st_dev,
> (unsigned long)last_ino);
> fd = openat(fd, "..", O_PATH | O_DIRECTORY | O_NOFOLLOW);
> level++;
> }
> }
>
>Before and after this patch only Level 0 is displayed. Without
>/proc/self/fd bind-mount protection it is possible to traverse parent
>directories.
>
>Fixes: 397ae982f4df4 ("virtiofsd: jail lo->proc_self_fd")
>Cc: Miklos Szeredi <mszeredi@redhat.com>
>Cc: Jens Freimann <jfreimann@redhat.com>
>Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Thanks Stefan, it fixes the problem we had!
Tested-by: Jens Freimann <jfreimann@redhat.com>
Reviewed-by: Jens Freimann <jfreimann@redhat.com>
regards,
Jens
next prev parent reply other threads:[~2020-10-06 13:43 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-06 9:58 [Virtio-fs] [PATCH] virtiofsd: avoid /proc/self/fd tempdir Stefan Hajnoczi
2020-10-06 9:58 ` Stefan Hajnoczi
2020-10-06 11:03 ` [Virtio-fs] " Dr. David Alan Gilbert
2020-10-06 11:03 ` Dr. David Alan Gilbert
2020-10-06 13:43 ` Jens Freimann [this message]
2020-10-06 13:43 ` Jens Freimann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201006134338.GA49600@linux.fritz.box \
--to=jfreimann@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
--cc=virtio-fs@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.