1. 07 3月, 2017 4 次提交
  2. 28 2月, 2017 30 次提交
    • G
      9pfs: local: drop unused code · c23d5f1d
      Greg Kurz 提交于
      Now that the all callbacks have been converted to use "at" syscalls, we
      can drop this code.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      c23d5f1d
    • G
      9pfs: local: open2: don't follow symlinks · a565fea5
      Greg Kurz 提交于
      The local_open2() callback is vulnerable to symlink attacks because it
      calls:
      
      (1) open() which follows symbolic links for all path elements but the
          rightmost one
      (2) local_set_xattr()->setxattr() which follows symbolic links for all
          path elements
      (3) local_set_mapped_file_attr() which calls in turn local_fopen() and
          mkdir(), both functions following symbolic links for all path
          elements but the rightmost one
      (4) local_post_create_passthrough() which calls in turn lchown() and
          chmod(), both functions also following symbolic links
      
      This patch converts local_open2() to rely on opendir_nofollow() and
      mkdirat() to fix (1), as well as local_set_xattrat(),
      local_set_mapped_file_attrat() and local_set_cred_passthrough() to
      fix (2), (3) and (4) respectively. Since local_open2() already opens
      a descriptor to the target file, local_set_cred_passthrough() is
      modified to reuse it instead of opening a new one.
      
      The mapped and mapped-file security modes are supposed to be identical,
      except for the place where credentials and file modes are stored. While
      here, we also make that explicit by sharing the call to openat().
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      a565fea5
    • G
      9pfs: local: mkdir: don't follow symlinks · 3f3a1699
      Greg Kurz 提交于
      The local_mkdir() callback is vulnerable to symlink attacks because it
      calls:
      
      (1) mkdir() which follows symbolic links for all path elements but the
          rightmost one
      (2) local_set_xattr()->setxattr() which follows symbolic links for all
          path elements
      (3) local_set_mapped_file_attr() which calls in turn local_fopen() and
          mkdir(), both functions following symbolic links for all path
          elements but the rightmost one
      (4) local_post_create_passthrough() which calls in turn lchown() and
          chmod(), both functions also following symbolic links
      
      This patch converts local_mkdir() to rely on opendir_nofollow() and
      mkdirat() to fix (1), as well as local_set_xattrat(),
      local_set_mapped_file_attrat() and local_set_cred_passthrough() to
      fix (2), (3) and (4) respectively.
      
      The mapped and mapped-file security modes are supposed to be identical,
      except for the place where credentials and file modes are stored. While
      here, we also make that explicit by sharing the call to mkdirat().
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      3f3a1699
    • G
      9pfs: local: mknod: don't follow symlinks · d815e721
      Greg Kurz 提交于
      The local_mknod() callback is vulnerable to symlink attacks because it
      calls:
      
      (1) mknod() which follows symbolic links for all path elements but the
          rightmost one
      (2) local_set_xattr()->setxattr() which follows symbolic links for all
          path elements
      (3) local_set_mapped_file_attr() which calls in turn local_fopen() and
          mkdir(), both functions following symbolic links for all path
          elements but the rightmost one
      (4) local_post_create_passthrough() which calls in turn lchown() and
          chmod(), both functions also following symbolic links
      
      This patch converts local_mknod() to rely on opendir_nofollow() and
      mknodat() to fix (1), as well as local_set_xattrat() and
      local_set_mapped_file_attrat() to fix (2) and (3) respectively.
      
      A new local_set_cred_passthrough() helper based on fchownat() and
      fchmodat_nofollow() is introduced as a replacement to
      local_post_create_passthrough() to fix (4).
      
      The mapped and mapped-file security modes are supposed to be identical,
      except for the place where credentials and file modes are stored. While
      here, we also make that explicit by sharing the call to mknodat().
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      d815e721
    • G
      9pfs: local: symlink: don't follow symlinks · 38771613
      Greg Kurz 提交于
      The local_symlink() callback is vulnerable to symlink attacks because it
      calls:
      
      (1) symlink() which follows symbolic links for all path elements but the
          rightmost one
      (2) open(O_NOFOLLOW) which follows symbolic links for all path elements but
          the rightmost one
      (3) local_set_xattr()->setxattr() which follows symbolic links for all
          path elements
      (4) local_set_mapped_file_attr() which calls in turn local_fopen() and
          mkdir(), both functions following symbolic links for all path
          elements but the rightmost one
      
      This patch converts local_symlink() to rely on opendir_nofollow() and
      symlinkat() to fix (1), openat(O_NOFOLLOW) to fix (2), as well as
      local_set_xattrat() and local_set_mapped_file_attrat() to fix (3) and
      (4) respectively.
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      38771613
    • G
      9pfs: local: chown: don't follow symlinks · d369f207
      Greg Kurz 提交于
      The local_chown() callback is vulnerable to symlink attacks because it
      calls:
      
      (1) lchown() which follows symbolic links for all path elements but the
          rightmost one
      (2) local_set_xattr()->setxattr() which follows symbolic links for all
          path elements
      (3) local_set_mapped_file_attr() which calls in turn local_fopen() and
          mkdir(), both functions following symbolic links for all path
          elements but the rightmost one
      
      This patch converts local_chown() to rely on open_nofollow() and
      fchownat() to fix (1), as well as local_set_xattrat() and
      local_set_mapped_file_attrat() to fix (2) and (3) respectively.
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      d369f207
    • G
      9pfs: local: chmod: don't follow symlinks · e3187a45
      Greg Kurz 提交于
      The local_chmod() callback is vulnerable to symlink attacks because it
      calls:
      
      (1) chmod() which follows symbolic links for all path elements
      (2) local_set_xattr()->setxattr() which follows symbolic links for all
          path elements
      (3) local_set_mapped_file_attr() which calls in turn local_fopen() and
          mkdir(), both functions following symbolic links for all path
          elements but the rightmost one
      
      We would need fchmodat() to implement AT_SYMLINK_NOFOLLOW to fix (1). This
      isn't the case on linux unfortunately: the kernel doesn't even have a flags
      argument to the syscall :-\ It is impossible to fix it in userspace in
      a race-free manner. This patch hence converts local_chmod() to rely on
      open_nofollow() and fchmod(). This fixes the vulnerability but introduces
      a limitation: the target file must readable and/or writable for the call
      to openat() to succeed.
      
      It introduces a local_set_xattrat() replacement to local_set_xattr()
      based on fsetxattrat() to fix (2), and a local_set_mapped_file_attrat()
      replacement to local_set_mapped_file_attr() based on local_fopenat()
      and mkdirat() to fix (3). No effort is made to factor out code because
      both local_set_xattr() and local_set_mapped_file_attr() will be dropped
      when all users have been converted to use the "at" versions.
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      e3187a45
    • G
      9pfs: local: link: don't follow symlinks · ad0b46e6
      Greg Kurz 提交于
      The local_link() callback is vulnerable to symlink attacks because it calls:
      
      (1) link() which follows symbolic links for all path elements but the
          rightmost one
      (2) local_create_mapped_attr_dir()->mkdir() which follows symbolic links
          for all path elements but the rightmost one
      
      This patch converts local_link() to rely on opendir_nofollow() and linkat()
      to fix (1), mkdirat() to fix (2).
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      ad0b46e6
    • G
      9pfs: local: improve error handling in link op · 6dd4b1f1
      Greg Kurz 提交于
      When using the mapped-file security model, we also have to create a link
      for the metadata file if it exists. In case of failure, we should rollback.
      
      That's what this patch does.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      6dd4b1f1
    • G
      9pfs: local: rename: use renameat · d2767ede
      Greg Kurz 提交于
      The local_rename() callback is vulnerable to symlink attacks because it
      uses rename() which follows symbolic links in all path elements but the
      rightmost one.
      
      This patch simply transforms local_rename() into a wrapper around
      local_renameat() which is symlink-attack safe.
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      d2767ede
    • G
      9pfs: local: renameat: don't follow symlinks · 99f2cf4b
      Greg Kurz 提交于
      The local_renameat() callback is currently a wrapper around local_rename()
      which is vulnerable to symlink attacks.
      
      This patch rewrites local_renameat() to have its own implementation, based
      on local_opendir_nofollow() and renameat().
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      99f2cf4b
    • G
      9pfs: local: lstat: don't follow symlinks · f9aef99b
      Greg Kurz 提交于
      The local_lstat() callback is vulnerable to symlink attacks because it
      calls:
      
      (1) lstat() which follows symbolic links in all path elements but the
          rightmost one
      (2) getxattr() which follows symbolic links in all path elements
      (3) local_mapped_file_attr()->local_fopen()->openat(O_NOFOLLOW) which
          follows symbolic links in all path elements but the rightmost
          one
      
      This patch converts local_lstat() to rely on opendir_nofollow() and
      fstatat(AT_SYMLINK_NOFOLLOW) to fix (1), fgetxattrat_nofollow() to
      fix (2).
      
      A new local_fopenat() helper is introduced as a replacement to
      local_fopen() to fix (3). No effort is made to factor out code
      because local_fopen() will be dropped when all users have been
      converted to call local_fopenat().
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      f9aef99b
    • G
      9pfs: local: readlink: don't follow symlinks · bec1e954
      Greg Kurz 提交于
      The local_readlink() callback is vulnerable to symlink attacks because it
      calls:
      
      (1) open(O_NOFOLLOW) which follows symbolic links for all path elements but
          the rightmost one
      (2) readlink() which follows symbolic links for all path elements but the
          rightmost one
      
      This patch converts local_readlink() to rely on open_nofollow() to fix (1)
      and opendir_nofollow(), readlinkat() to fix (2).
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      bec1e954
    • G
      9pfs: local: truncate: don't follow symlinks · ac125d99
      Greg Kurz 提交于
      The local_truncate() callback is vulnerable to symlink attacks because
      it calls truncate() which follows symbolic links in all path elements.
      
      This patch converts local_truncate() to rely on open_nofollow() and
      ftruncate() instead.
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      ac125d99
    • G
      9pfs: local: statfs: don't follow symlinks · 31e51d1c
      Greg Kurz 提交于
      The local_statfs() callback is vulnerable to symlink attacks because it
      calls statfs() which follows symbolic links in all path elements.
      
      This patch converts local_statfs() to rely on open_nofollow() and fstatfs()
      instead.
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      31e51d1c
    • G
      9pfs: local: utimensat: don't follow symlinks · a33eda0d
      Greg Kurz 提交于
      The local_utimensat() callback is vulnerable to symlink attacks because it
      calls qemu_utimens()->utimensat(AT_SYMLINK_NOFOLLOW) which follows symbolic
      links in all path elements but the rightmost one or qemu_utimens()->utimes()
      which follows symbolic links for all path elements.
      
      This patch converts local_utimensat() to rely on opendir_nofollow() and
      utimensat(AT_SYMLINK_NOFOLLOW) directly instead of using qemu_utimens().
      It is hence assumed that the OS supports utimensat(), i.e. has glibc 2.6
      or higher and linux 2.6.22 or higher, which seems reasonable nowadays.
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      a33eda0d
    • G
      9pfs: local: remove: don't follow symlinks · a0e640a8
      Greg Kurz 提交于
      The local_remove() callback is vulnerable to symlink attacks because it
      calls:
      
      (1) lstat() which follows symbolic links in all path elements but the
          rightmost one
      (2) remove() which follows symbolic links in all path elements but the
          rightmost one
      
      This patch converts local_remove() to rely on opendir_nofollow(),
      fstatat(AT_SYMLINK_NOFOLLOW) to fix (1) and unlinkat() to fix (2).
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      a0e640a8
    • G
      9pfs: local: unlinkat: don't follow symlinks · df4938a6
      Greg Kurz 提交于
      The local_unlinkat() callback is vulnerable to symlink attacks because it
      calls remove() which follows symbolic links in all path elements but the
      rightmost one.
      
      This patch converts local_unlinkat() to rely on opendir_nofollow() and
      unlinkat() instead.
      
      Most of the code is moved to a separate local_unlinkat_common() helper
      which will be reused in a subsequent patch to fix the same issue in
      local_remove().
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      df4938a6
    • G
      9pfs: local: lremovexattr: don't follow symlinks · 72f0d0bf
      Greg Kurz 提交于
      The local_lremovexattr() callback is vulnerable to symlink attacks because
      it calls lremovexattr() which follows symbolic links in all path elements
      but the rightmost one.
      
      This patch introduces a helper to emulate the non-existing fremovexattrat()
      function: it is implemented with /proc/self/fd which provides a trusted
      path that can be safely passed to lremovexattr().
      
      local_lremovexattr() is converted to use this helper and opendir_nofollow().
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      72f0d0bf
    • G
      9pfs: local: lsetxattr: don't follow symlinks · 3e36aba7
      Greg Kurz 提交于
      The local_lsetxattr() callback is vulnerable to symlink attacks because
      it calls lsetxattr() which follows symbolic links in all path elements but
      the rightmost one.
      
      This patch introduces a helper to emulate the non-existing fsetxattrat()
      function: it is implemented with /proc/self/fd which provides a trusted
      path that can be safely passed to lsetxattr().
      
      local_lsetxattr() is converted to use this helper and opendir_nofollow().
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      3e36aba7
    • G
      9pfs: local: llistxattr: don't follow symlinks · 5507904e
      Greg Kurz 提交于
      The local_llistxattr() callback is vulnerable to symlink attacks because
      it calls llistxattr() which follows symbolic links in all path elements but
      the rightmost one.
      
      This patch introduces a helper to emulate the non-existing flistxattrat()
      function: it is implemented with /proc/self/fd which provides a trusted
      path that can be safely passed to llistxattr().
      
      local_llistxattr() is converted to use this helper and opendir_nofollow().
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      5507904e
    • G
      9pfs: local: lgetxattr: don't follow symlinks · 56ad3e54
      Greg Kurz 提交于
      The local_lgetxattr() callback is vulnerable to symlink attacks because
      it calls lgetxattr() which follows symbolic links in all path elements but
      the rightmost one.
      
      This patch introduces a helper to emulate the non-existing fgetxattrat()
      function: it is implemented with /proc/self/fd which provides a trusted
      path that can be safely passed to lgetxattr().
      
      local_lgetxattr() is converted to use this helper and opendir_nofollow().
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      56ad3e54
    • G
      9pfs: local: open/opendir: don't follow symlinks · 996a0d76
      Greg Kurz 提交于
      The local_open() and local_opendir() callbacks are vulnerable to symlink
      attacks because they call:
      
      (1) open(O_NOFOLLOW) which follows symbolic links in all path elements but
          the rightmost one
      (2) opendir() which follows symbolic links in all path elements
      
      This patch converts both callbacks to use new helpers based on
      openat_nofollow() to only open files and directories if they are
      below the virtfs shared folder
      
      This partly fixes CVE-2016-9602.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      996a0d76
    • G
      9pfs: local: keep a file descriptor on the shared folder · 0e35a378
      Greg Kurz 提交于
      This patch opens the shared folder and caches the file descriptor, so that
      it can be used to do symlink-safe path walk.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      0e35a378
    • G
      9pfs: introduce relative_openat_nofollow() helper · 6482a961
      Greg Kurz 提交于
      When using the passthrough security mode, symbolic links created by the
      guest are actual symbolic links on the host file system.
      
      Since the resolution of symbolic links during path walk is supposed to
      occur on the client side. The server should hence never receive any path
      pointing to an actual symbolic link. This isn't guaranteed by the protocol
      though, and malicious code in the guest can trick the server to issue
      various syscalls on paths whose one or more elements are symbolic links.
      In the case of the "local" backend using the "passthrough" or "none"
      security modes, the guest can directly create symbolic links to arbitrary
      locations on the host (as per spec). The "mapped-xattr" and "mapped-file"
      security modes are also affected to a lesser extent as they require some
      help from an external entity to create actual symbolic links on the host,
      i.e. another guest using "passthrough" mode for example.
      
      The current code hence relies on O_NOFOLLOW and "l*()" variants of system
      calls. Unfortunately, this only applies to the rightmost path component.
      A guest could maliciously replace any component in a trusted path with a
      symbolic link. This could allow any guest to escape a virtfs shared folder.
      
      This patch introduces a variant of the openat() syscall that successively
      opens each path element with O_NOFOLLOW. When passing a file descriptor
      pointing to a trusted directory, one is guaranteed to be returned a
      file descriptor pointing to a path which is beneath the trusted directory.
      This will be used by subsequent patches to implement symlink-safe path walk
      for any access to the backend.
      
      Symbolic links aren't the only threats actually: a malicious guest could
      change a path element to point to other types of file with undesirable
      effects:
      - a named pipe or any other thing that would cause openat() to block
      - a terminal device which would become QEMU's controlling terminal
      
      These issues can be addressed with O_NONBLOCK and O_NOCTTY.
      
      Two helpers are introduced: one to open intermediate path elements and one
      to open the rightmost path element.
      Suggested-by: NJann Horn <jannh@google.com>
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      (renamed openat_nofollow() to relative_openat_nofollow(),
       assert path is relative and doesn't contain '//',
       fixed side-effect in assert, Greg Kurz)
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      6482a961
    • G
      9pfs: remove side-effects in local_open() and local_opendir() · 21328e1e
      Greg Kurz 提交于
      If these functions fail, they should not change *fs. Let's use local
      variables to fix this.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      21328e1e
    • G
      9pfs: remove side-effects in local_init() · 00c90bd1
      Greg Kurz 提交于
      If this function fails, it should not modify *ctx.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      00c90bd1
    • G
      9pfs: local: move xattr security ops to 9p-xattr.c · 56fc494b
      Greg Kurz 提交于
      These functions are always called indirectly. It really doesn't make sense
      for them to sit in a header file.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      56fc494b
    • P
      fsdev: add IO throttle support to fsdev devices · b8bbdb88
      Pradeep Jagadeesh 提交于
      This patchset adds the throttle support for the 9p-local driver.
      For now this functionality can be enabled only through qemu cli options.
      QMP interface and support to other drivers need further extensions.
      To make it simple for other 9p drivers, the throttle code has been put in
      separate files.
      Signed-off-by: NPradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
      Reviewed-by: NAlberto Garcia <berto@igalia.com>
      (pass extra NULL CoMutex * argument to qemu_co_queue_wait(),
       added options to qemu-options.hx, Greg Kurz)
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      b8bbdb88
    • P
      9pfs: fix v9fs_lock error case · 4bae2b39
      Paolo Bonzini 提交于
      In this case, we are marshaling an error status instead of the errno value.
      Reorganize the out and out_nofid labels to look like all the other cases.
      Coverity reports this because the "err = -ENOENT" and "err = -EINVAL"
      assignments above are dead, overwritten by the call to pdu_marshal.
      
      (Coverity issues CID1348512 and CID1348513)
      Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
      (also open-coded the success path since locking is a nop for us, Greg Kurz)
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      4bae2b39
  3. 21 2月, 2017 1 次提交
  4. 25 1月, 2017 5 次提交