1. 24 12月, 2015 10 次提交
  2. 13 12月, 2015 1 次提交
    • E
      CVE-2015-5313: storage: don't allow '/' in filesystem volume names · b553ec76
      Eric Blake 提交于
      The libvirt file system storage driver determines what file to
      act on by concatenating the pool location with the volume name.
      If a user is able to pick names like "../../../etc/passwd", then
      they can escape the bounds of the pool.  For that matter,
      virStoragePoolListVolumes() doesn't descend into subdirectories,
      so a user really shouldn't use a name with a slash.
      
      Normally, only privileged users can coerce libvirt into creating
      or opening existing files using the virStorageVol APIs; and such
      users already have full privilege to create any domain XML (so it
      is not an escalation of privilege).  But in the case of
      fine-grained ACLs, it is feasible that a user can be granted
      storage_vol:create but not domain:write, and it violates
      assumptions if such a user can abuse libvirt to access files
      outside of the storage pool.
      
      Therefore, prevent all use of volume names that contain "/",
      whether or not such a name is actually attempting to escape the
      pool.
      
      This changes things from:
      
      $ virsh vol-create-as default ../../../../../../etc/haha --capacity 128
      Vol ../../../../../../etc/haha created
      $ rm /etc/haha
      
      to:
      
      $ virsh vol-create-as default ../../../../../../etc/haha --capacity 128
      error: Failed to create vol ../../../../../../etc/haha
      error: Requested operation is not valid: volume name '../../../../../../etc/haha' cannot contain '/'
      Signed-off-by: NEric Blake <eblake@redhat.com>
      (cherry picked from commit 034e47c3)
      b553ec76
  3. 03 9月, 2015 1 次提交
    • M
      remoteClientCloseFunc: Don't mangle connection object refcount · 175f5022
      Michal Privoznik 提交于
      Well, in 8ad126e6 we tried to fix a memory corruption problem.
      However, the fix was not as good as it could be. I mean, the
      commit has one line more than it should. I've noticed this output
      just recently:
      
        # ./run valgrind --leak-check=full --show-reachable=yes ./tools/virsh domblklist gentoo
        ==17019== Memcheck, a memory error detector
        ==17019== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
        ==17019== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
        ==17019== Command: /home/zippy/work/libvirt/libvirt.git/tools/.libs/virsh domblklist gentoo
        ==17019==
        Target     Source
        ------------------------------------------------
        fda        /var/lib/libvirt/images/fd.img
        vda        /var/lib/libvirt/images/gentoo.qcow2
        hdc        /home/zippy/tmp/install-amd64-minimal-20150402.iso
      
        ==17019== Thread 2:
        ==17019== Invalid read of size 4
        ==17019==    at 0x4EFF5B4: virObjectUnref (virobject.c:258)
        ==17019==    by 0x5038CFF: remoteClientCloseFunc (remote_driver.c:552)
        ==17019==    by 0x5069D57: virNetClientCloseLocked (virnetclient.c:685)
        ==17019==    by 0x506C848: virNetClientIncomingEvent (virnetclient.c:1852)
        ==17019==    by 0x5082136: virNetSocketEventHandle (virnetsocket.c:1913)
        ==17019==    by 0x4ECD64E: virEventPollDispatchHandles (vireventpoll.c:509)
        ==17019==    by 0x4ECDE02: virEventPollRunOnce (vireventpoll.c:658)
        ==17019==    by 0x4ECBF00: virEventRunDefaultImpl (virevent.c:308)
        ==17019==    by 0x130386: vshEventLoop (vsh.c:1864)
        ==17019==    by 0x4F1EB07: virThreadHelper (virthread.c:206)
        ==17019==    by 0xA8462D3: start_thread (in /lib64/libpthread-2.20.so)
        ==17019==    by 0xAB441FC: clone (in /lib64/libc-2.20.so)
        ==17019==  Address 0x139023f4 is 4 bytes inside a block of size 240 free'd
        ==17019==    at 0x4C2B1F0: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
        ==17019==    by 0x4EA8949: virFree (viralloc.c:582)
        ==17019==    by 0x4EFF6D0: virObjectUnref (virobject.c:273)
        ==17019==    by 0x4FE74D6: virConnectClose (libvirt.c:1390)
        ==17019==    by 0x13342A: virshDeinit (virsh.c:406)
        ==17019==    by 0x134A37: main (virsh.c:950)
      
      The problem is, when registering remoteClientCloseFunc(), it's
      conn->closeCallback which is ref'd. But in the function itself
      it's conn->closeCallback->conn what is unref'd. This is causing
      imbalance in reference counting. Moreover, there's no need for
      the remote driver to increase/decrease conn refcount since it's
      not used anywhere. It's just merely passed to client registered
      callback. And for that purpose it's correctly ref'd in
      virConnectRegisterCloseCallback() and then unref'd in
      virConnectUnregisterCloseCallback().
      Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
      (cherry picked from commit e6893007)
      Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
      175f5022
  4. 29 8月, 2015 1 次提交
  5. 02 7月, 2015 1 次提交
    • M
      lxc: Don't pass a local variable address randomly · 1f15a5d4
      Michal Privoznik 提交于
      So, recently I was testing the LXC driver. You know, startup some
      domains. But to my surprise, I was not able to start a single one:
      
        virsh # start --console test
        error: Reconnected to the hypervisor
        error: Failed to start domain test
        error: internal error: guest failed to start: unexpected exit status 125
      
      So I've start digging. It turns out, that in virExec(), when I printed
      out the @cmd, I got strange values: *(cmd->outfdptr) was certainly not
      valid FD number: it has random value of several millions. This
      obviously made prepareStdFd(childout, STDOUT_FILENO) fail (line 611).
      But outfdptr is set in virCommandSetOutputFD(). The only place within
      LXC driver where the function is called is in
      virLXCProcessBuildControllerCmd(). If you take a closer look at the
      function it looks like this:
      
      static virCommandPtr
      virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
                                      ..
                                      int logfd,
                                      const char *pidfile)
      {
          ...
          virCommandSetOutputFD(cmd, &logfd);
          virCommandSetErrorFD(cmd, &logfd);
          ...
      }
      
      Yes, you guessed it. @logfd is passed into the function by value.
      However, in the function we try to get its address (an address of a
      local variable) which is no longer valid once function is finished and
      stack is cleaned. Therefore when cmd->outfdptr is evaluated at any
      point after this function, we may get a random number, depending on
      what's currently on the stack. Of course, this may work sometimes too
      - it depends on the compiler how it arranges the code, when the stack
      is wiped out.
      
      In order to fix this, lets pass a pointer to @logfd instead of
      figuring out (wrong) its value in a function.
      
      The bug was introduced in e1de5521.
      Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
      (cherry picked from commit 302146b1)
      1f15a5d4
  6. 17 6月, 2015 1 次提交
  7. 06 6月, 2015 2 次提交
    • L
      virnetdev: fix moving of 802.11 phys · d7395473
      Lubomir Rintel 提交于
      There was a couple of problems with the style fixes applied to the original
      patch:
      
      1.) virFileReadAllQuiet comparison was incorrectly parenthesized when moved
      into a condition, causing the len to be set to the result of comparison. This,
      together with the removed underflow check would underflow the phy buffer.
      
      2.) The logic was broken. Failure to call "ip" would abort the function, thus
      the "iw" branch would never be reached.
      
      This aims to fix the issues and work around possible style complains :)
      Signed-off-by: NLubomir Rintel <lkundrak@v3.sk>
      (cherry picked from commit 81b19ce4)
      d7395473
    • L
      interface: don't error out if a bond has no interfaces · cc2add96
      Lubomir Rintel 提交于
      It's not a problem at all and causes virt-manager to break down.
      
      Note: netcf 0.2.8 and earlier generates invalid XML for a bond with no
      interfaces anyway, so in that case this error in libvirt is never
      reached since we fail earlier.
      Signed-off-by: NLubomir Rintel <lkundrak@v3.sk>
      (cherry picked from commit efc68de5)
      cc2add96
  8. 28 5月, 2015 6 次提交
  9. 28 4月, 2015 17 次提交
    • C
      Prep for release 1.2.13.1 · 0911d1c6
      Cole Robinson 提交于
      0911d1c6
    • J
      Fix memory leak in virNetSocketNewConnectUNIX · 40d774a4
      Jiri Denemark 提交于
      ==26726==    by 0x673CD67: __vasprintf_chk (vasprintf_chk.c:80)
      ==26726==    by 0x5673605: UnknownInlinedFun (stdio2.h:210)
      ==26726==    by 0x5673605: virVasprintfInternal (virstring.c:476)
      ==26726==    by 0x56736EE: virAsprintfInternal (virstring.c:497)
      ==26726==    by 0x5680C37: virGetUserRuntimeDirectory (virutil.c:866)
      ==26726==    by 0x5783A89: virNetSocketNewConnectUNIX (virnetsocket.c:572)
      ==26726==    by 0x57751AF: virNetClientNewUNIX (virnetclient.c:344)
      ==26726==    by 0x57689B3: doRemoteOpen (remote_driver.c:895)
      ==26726==    by 0x5769F8E: remoteConnectOpen (remote_driver.c:1195)
      ==26726==    by 0x57092DF: do_open (libvirt.c:1189)
      ==26726==    by 0x570A7BF: virConnectOpenAuth (libvirt.c:1341)
      
      https://bugzilla.redhat.com/show_bug.cgi?id=1215042Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
      (cherry picked from commit da4d7c30)
      40d774a4
    • D
      rng: fix port number range validation · 9117783b
      Daniel P. Berrange 提交于
      The PortNumber data type is declared to derive from 'short'.
      Unfortunately this is an signed type, so validates the range
      [-32,768, 32,767] which excludes valid port numbers between
      32767 and 65535.
      
      We can't use 'unsignedShort', since we need -1 to be a valid
      port number too.
      
      This change is to use 'int' and set an explicit max boundary
      instead of relying on the data types' built-in max.
      
      One of the existing tests is changed to use a high port number
      to validate the schema.
      
      https://bugzilla.redhat.com/show_bug.cgi?id=1214664Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      (cherry picked from commit 615bdfda)
      9117783b
    • Z
      qemu: Don't fail to reboot domains with unresponsive agent · f708713e
      zhang bo 提交于
      just as what b8e25c35 did, we
      fall back to the ACPI method when the guest agent is unresponsive
      in qemuDomainReboot().
      Signed-off-by: NYueWenyuan <yuewenyuan@huawei.com>
      Signed-off-by: NZhang Bo <oscar.zhangbo@huawei.com>
      Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
      (cherry picked from commit eadf41fe)
      f708713e
    • R
      vircommand: fix polling in virCommandProcessIO · b5a210dc
      Roman Bogorodskiy 提交于
      When running on FreeBSD, there's a bug in virCommandProcessIO
      polling that is triggered by the commandtest.
      
      A test that triggers EPIPE in commandtest (named "test20") hungs
      forever on FreeBSD.
      
      Apparently, this happens because FreeBSD sets POLLHUP flag on revents
      when stdin in closed. And as the current implementation only checks for
      POLLOUT and POLLERR, it ends up looping forever inside
      virCommandProcessIO and not trying to do one more write() that would
      trigger EPIPE.
      
      To fix that check for the POLLHUP flag along with POLLOUT and POLLERR.
      
      (cherry picked from commit e34cccf7)
      b5a210dc
    • P
      util: storage: Fix possible crash when source path is NULL · e84e5aa3
      Peter Krempa 提交于
      Some storage protocols allow to have the @path field in struct
      virStorageSource set to NULL. Add NULLSTR() wrappers to handle this
      possibility until I finish the storage source error formatter.
      
      (cherry picked from commit 62a61d58)
      e84e5aa3
    • L
      qemu: set macvtap physdevs online when macvtap is set online · 24e0cecb
      Laine Stump 提交于
      A further fix for:
      
        https://bugzilla.redhat.com/show_bug.cgi?id=1113474
      
      Since there is no possibility that any type of macvtap will work if
      the parent physdev it's attached to is offline, we should bring the
      physdev online at the same time as the macvtap. When taking the
      macvtap offline, it's also necessary to take the physdev offline for
      macvtap passthrough mode (because the physdev has the same MAC address
      as the macvtap device, so could potentially cause problems with
      misdirected packets during migration, as outlined in commits 829770
      and 879c13). We can't set the physdev offline for other macvtap modes
      1) because there may be other macvtap devices attached to the same
      physdev (and/or the host itself may be using the device) in the other
      modes whereas passthrough mode is exclusive to one macvtap at a time,
      and 2) there's no practical reason to do so anyway.
      
      (cherry picked from commit 38172ed8)
      24e0cecb
    • L
      util: set MAC address for VF via netlink message to PF+VF# when possible · d336362c
      Laine Stump 提交于
      Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1113474
      
      When we set the MAC address of a network device as a part of setting
      up macvtap "passthrough" mode (where the domain has an emulated netdev
      connected to a host macvtap device that has exclusive use of the
      physical device, and sets the device MAC address to match its own,
      i.e. "<interface type='direct'> <source mode='passthrough' .../>"), we
      use ioctl(SIOCSIFHWADDR) giving it the name of that device. This is
      true even if it is an SRIOV Virtual Function (VF).
      
      But, when we are setting the MAC address / vlan ID of a VF in
      preparation for "hostdev network" passthrough (this is where we set
      the MAC address and vlan id of the VF after detaching the host net
      driver and before assigning the device to the domain with PCI
      passthrough, i.e. "<interface type='hostdev'>", we do the setting via
      a netlink RTM_SETLINK message for that VF's Physical Function (PF),
      telling it the VF# we want to change. This sets an "administratively
      changed MAC" flag for that VF in the PF's driver, and from that point
      on (until the PF driver is reloaded, *not* merely the VF driver) that
      VF's MAC address can't be changed using ioctl(SIOCSIFHWADDR) - the
      only way to change it is via the PF with RTM_SETLINK.
      
      This means that if a VF is used for hostdev passthrough, it will have
      the admin flag set, and future attempts to use that VF for macvtap
      passthrough will fail.
      
      The solution to this problem is to check if the device being used for
      macvtap passthrough is actually a VF; if so, we use the netlink
      RTM_SETLINK message to the PF to set the VF's mac address instead of
      ioctl(SIOCSIFHWADDR) directly to the VF; if not, behavior does not
      change from previously.
      
      There are three pieces to making this work:
      
      1) virNetDevMacVLan(Create|Delete)WithVPortProfile() now call
         virNetDev(Replace|Restore)NetConfig() rather than
         virNetDev(Replace|Restore)MacAddress() (simply passing -1 for VF#
         and vlanid).
      
      2) virNetDev(Replace|Restore)NetConfig() check to see if the device is
         a VF. If so, they find the PF's name and VF#, allowing them to call
         virNetDev(Replace|Restore)VfConfig().
      
      3) To prevent mixups when detaching a macvtap passthrough device that
         had been attached while running an older version of libvirt,
         virNetDevRestoreVfConfig() is potentially given the preserved name
         of the VF, and if the proper statefile for a VF can't be found in
         the stateDir (${stateDir}/${pfname}_vf${vfid}),
         virNetDevRestoreMacAddress() is called instead (which will look in
         the file named ${stateDir}/${vfname}).
      
      This problem has existed in every version of libvirt that has both
      macvtap passthrough and interface type='hostdev'. Fortunately people
      seem to use one or the other though, so it hasn't caused any real
      world problem reports.
      
      (cherry picked from commit cb3fe38c)
      d336362c
    • R
      xend: Remove a couple of unused function prototypes. · d1ef7b62
      Richard W.M. Jones 提交于
      Commit 70f44663 (from 2008) introduced
      some functions for testing whether xend was returning correct sound
      models.  Those functions have long gone, but the function prototypes
      remain.  This commit removes the unused prototypes.
      Signed-off-by: NRichard W.M. Jones <rjones@redhat.com>
      (cherry picked from commit 093eea95)
      d1ef7b62
    • Z
      qemuDomainShutdownFlags: Set fakeReboot more frequently · 0636291d
      zhang bo 提交于
      When a qemu domain is to be rebooted, from outside, at libvirt
      level it looks like regular shutdown. To really restart the
      domain, libvirt needs to issue reset command on the monitor once
      SHUTDOWN event appeared. So, in order to differentiate bare
      shutdown and reboot libvirt uses a variable within domain private
      data. It's called fakeReboot. When the reboot API is called, the
      variable is set, but when the shutdown API is called it must be
      cleared out. But it was not for every possible case. So if user
      called virDomainReboot(), and there was no ACPI daemon running
      inside the guest (so guest didn't initiated shutdown sequence)
      and then virDomainShutdown(mode=agent) was called bad thing
      happened. We remembered the fakeReboot and instead of shutting
      the domain down, we just rebooted it.
      Signed-off-by: NZhang Bo <oscar.zhangbo@huawei.com>
      Signed-off-by: NWang Yufei <james.wangyufei@huawei.com>
      Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
      (cherry picked from commit 8be502fd)
      0636291d
    • M
      nwfilter: Partly initialize driver even for non-privileged users · 10736cb4
      Michal Privoznik 提交于
      https://bugzilla.redhat.com/show_bug.cgi?id=1211436
      
      This reverts commit b7829f95.
      
      The previous fix was not correct. Like everywhere else, a driver is a
      global variable allocated in stateInitialize function (or something
      similar for stateless drivers). Later, when a driver API is called,
      it's possible that the global variable is accessed and dereferenced.
      Now, some drivers require root privileges because they undertake some
      actions reserved only for the system admin (e.g. manipulating host
      firewall). And here's the trouble, the NWFilter state initializer
      exited too early when finding out it's running unprivileged, leaving
      the global NWFilter driver variable uninitialized. Any subsequent
      API call that tried to lock the driver resulted in dereferencing the
      driver and thus crash.
      
      On the other hand, in order to not resurrect the bug the original
      commit was fixing, Let's forbid the nwfilter define in session mode.
      Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
      
      Conflicts:
      	src/nwfilter/nwfilter_driver.c: Context. Code changed a bit
              since 2013.
      
      (cherry picked from commit 77d92e2e)
      10736cb4
    • M
      virNetSocketNewConnectUNIX: Don't unlink(NULL) · 5328780b
      Michal Privoznik 提交于
      There is a possibility that we jump onto error label with @lockpath
      still initialized to NULL. Here, the @lockpath should be unlink()-ed,
      but passing there a NULL is not a good idea. Don't do that. In fact,
      we should call unlink() only if we created the lock file successfully.
      Reported-by: NJohn Ferlan <jferlan@redhat.com>
      Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
      (cherry picked from commit 1fdac3d9)
      5328780b
    • J
      sanlock: Use VIR_ERR_RESOURCE_BUSY if sanlock_acquire fails · c6e7a9ea
      Jiri Denemark 提交于
      When acquiring resource via sanlock fails, we would report it as
      VIR_ERR_INTERNAL_ERROR, which is not very friendly to applications using
      libvirt. Moreover, the lockd driver would report the same failure as
      VIR_ERR_RESOURCE_BUSY, which looks better.
      
      Unfortunately, in sanlock driver we don't really know if acquiring the
      resource failed because it was already locked or there was another
      reason behind. But the end result is the same and I think using
      VIR_ERR_RESOURCE_BUSY reason for all acquire failures is still better
      than what we have now.
      
      https://bugzilla.redhat.com/show_bug.cgi?id=1165119Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
      (cherry picked from commit 4864e377)
      c6e7a9ea
    • M
      qemuMigrationPrecreateStorage: Fix debug message · 9d0c2053
      Michal Privoznik 提交于
      When pre-creating storage for domains, we need to find corresponding
      disk in the XML on the destination (domain XML may differ there, e.g.
      disk is accessible under different path). For better debugging, I'm
      printing all info I received on a disk. But there was a typo when
      printing the disk capacity: "%lluu" instead of "%llu".
      Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
      (cherry picked from commit 65a88572)
      9d0c2053
    • X
      qemu_migration.c: sleep first before checking for migration status. · 8ab12f1a
      Xing Lin 提交于
      The problem with the previous implementation is,
      even when qemuMigrationUpdateJobStatus() detects a migration job
      has completed, it will do a sleep for 50 ms (which is unnecessary
      and only adds up to the VM pause time).
      Signed-off-by: NXing Lin <xinglin@cs.utah.edu>
      Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
      (cherry picked from commit 522e81cb)
      8ab12f1a
    • M
      qemu_driver: check caps after starting block job · d17b2d9d
      Michael Chapman 提交于
      Currently we check qemuCaps before starting the block job. But qemuCaps
      isn't available on a stopped domain, which means we get a misleading
      error message in this case:
      
        # virsh domstate example
        shut off
      
        # virsh blockjob example vda
        error: unsupported configuration: block jobs not supported with this QEMU binary
      
      Move the qemuCaps check into the block job so that we are guaranteed the
      domain is running.
      Signed-off-by: NMichael Chapman <mike@very.puzzling.org>
      (cherry picked from commit cfcdf5ff)
      d17b2d9d
    • M
      qemu_migrate: use nested job when adding NBD to cookie · 0ff86a47
      Michael Chapman 提交于
      qemuMigrationCookieAddNBD is usually called from within an async
      MIGRATION_OUT or MIGRATION_IN job, so it needs to start a nested job.
      
      (The one exception is during the Begin phase when change protection
      isn't enabled, but qemuDomainObjEnterMonitorAsync will behave the same
      as qemuDomainObjEnterMonitor in this case.)
      
      This bug was encountered with a libvirt client that repeatedly queries
      the disk mirroring block job info during a migration. If one of these
      queries occurs just as the Perform migration cookie is baked, libvirt
      crashes.
      
      Relevant logs are as follows:
      
          6701: warning : qemuDomainObjEnterMonitorInternal:1544 : This thread seems to be the async job owner; entering monitor without asking for a nested job is dangerous
      [1] 6701: info : qemuMonitorSend:972 : QEMU_MONITOR_SEND_MSG: mon=0x7fefdc004700 msg={"execute":"query-block","id":"libvirt-629"}
      [2] 6699: info : qemuMonitorIOWrite:503 : QEMU_MONITOR_IO_WRITE: mon=0x7fefdc004700 buf={"execute":"query-block","id":"libvirt-629"}
      [3] 6704: info : qemuMonitorSend:972 : QEMU_MONITOR_SEND_MSG: mon=0x7fefdc004700 msg={"execute":"query-block-jobs","id":"libvirt-630"}
      [4] 6699: info : qemuMonitorJSONIOProcessLine:203 : QEMU_MONITOR_RECV_REPLY: mon=0x7fefdc004700 reply={"return": [...], "id": "libvirt-629"}
          6699: error : qemuMonitorJSONIOProcessLine:211 : internal error: Unexpected JSON reply '{"return": [...], "id": "libvirt-629"}'
      
      At [1] qemuMonitorBlockStatsUpdateCapacity sends its request, then waits
      on mon->notify. At [2] the request is written out to the monitor socket.
      At [3] qemuMonitorBlockJobInfo sends its request, and also waits on
      mon->notify. The reply from the first request is received at [4].
      However, qemuMonitorJSONIOProcessLine is not expecting this reply since
      the second request hadn't completed sending. The reply is dropped and an
      error is returned.
      
      qemuMonitorIO signals mon->notify twice during its error handling,
      waking up both of the threads waiting on it. One of them clears mon->msg
      as it exits qemuMonitorSend; the other crashes:
      
        qemuMonitorSend (mon=0x7fefdc004700, msg=<value optimized out>) at qemu/qemu_monitor.c:975
        975         while (!mon->msg->finished) {
        (gdb) print mon->msg
        $1 = (qemuMonitorMessagePtr) 0x0
      Signed-off-by: NMichael Chapman <mike@very.puzzling.org>
      (cherry picked from commit 72df8314)
      0ff86a47