1. 08 Feb, 2019 8 commits
    • Andre Przywara's avatar
      net/dhcp: avoid misleading strncpy · 0796825e
      Andre Przywara authored
      
      
      The code for copying an empty IP address into the DHCP opt buffer used
      strncpy, however used the source length as the size argument. GCC 8.x
      complains about it.
      
      Since the source string is actually fixed, just revert to the old
      strcpy, which gives us actually the same level of security in this case,
      but makes the compiler happy.
      Signed-off-by: Andre Przywara's avatarAndre Przywara <andre.przywara@arm.com>
      Signed-off-by: default avatarWill Deacon <will.deacon@arm.com>
      0796825e
    • Andre Przywara's avatar
      virtio: use strlcpy · 05755b29
      Andre Przywara authored
      
      
      GCC 8.x complains about improper usage of strncpy in virtio/net.c and
      virtio/scsi.c:
      In function 'virtio_scsi_init_one',
          inlined from 'virtio_scsi_init' at virtio/scsi.c:285:7:
      virtio/scsi.c:247:2: error: 'strncpy' specified bound 224 equals destination size [-Werror=stringop-truncation]
        strncpy((char *)&sdev->target.vhost_wwpn, disk->wwpn, sizeof(sdev->target.vhost_wwpn));
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      
      Fix this and the other occurences in virtio/ by using strlcpy instead
      of strncpy.
      Signed-off-by: Andre Przywara's avatarAndre Przywara <andre.przywara@arm.com>
      Signed-off-by: default avatarWill Deacon <will.deacon@arm.com>
      05755b29
    • Andre Przywara's avatar
      builtin-run: Replace strncpy calls with strlcpy · 266a0ed4
      Andre Przywara authored
      
      
      There are two uses of strncpy in builtin-run.c, where we don't make
      proper use of strncpy, so that GCC 8.x complains and aborts compilation.
      
      Replace those two calls with strlcpy(), which does the right thing in
      our case.
      Signed-off-by: Andre Przywara's avatarAndre Przywara <andre.przywara@arm.com>
      Signed-off-by: default avatarWill Deacon <will.deacon@arm.com>
      266a0ed4
    • Andre Przywara's avatar
      Makefile: support -s switch · 5eb1f27a
      Andre Przywara authored
      
      
      "make -s" suppresses normal output, just shows warnings and errors.
      But since we explicitly override the make output with our fancy concise
      version, we miss out on this feature.
      
      Do as the kernel does and explicitly suppress every normal output when -s
      is given. This helps to spot warnings that scroll out of the terminal
      window too quickly.
      Signed-off-by: Andre Przywara's avatarAndre Przywara <andre.przywara@arm.com>
      Signed-off-by: default avatarWill Deacon <will.deacon@arm.com>
      5eb1f27a
    • Andre Przywara's avatar
      arm: fdt: add stdout-path to /chosen node · 56e45ea4
      Andre Przywara authored
      
      
      The DT spec describes the stdout-path property in the /chosen node to
      contain the DT path for a default device usable for outputting characters.
      The Linux kernel uses this for earlycon (without further parameters),
      other DT users might rely on this as well.
      
      Add a stdout-path property pointing to the "serial0" alias, then add an
      aliases node at the end of the FDT, containing the actual path. This
      allows the FDT generation code in hw/serial.c to set this string.
      
      Even when we use the virtio console, the serial console is still there
      and works, so we can expose this unconditionally. Putting the virtio
      console path in there will not work anyway.
      Signed-off-by: Andre Przywara's avatarAndre Przywara <andre.przywara@arm.com>
      Signed-off-by: default avatarWill Deacon <will.deacon@arm.com>
      56e45ea4
    • Anisse Astier's avatar
      kvmtool: 9p: fix overapping snprintf · 04d604b6
      Anisse Astier authored
      
      
      GCC 8.2 gives this warning:
      
      virtio/9p.c: In function ‘virtio_p9_create’:
      virtio/9p.c:335:21: error: passing argument 1 to restrict-qualified parameter aliases with argument 4 [-Werror=restrict]
        ret = snprintf(dfid->path, size, "%s/%s", dfid->path, name);
                       ~~~~^~~~~~                 ~~~~~~~~~~
      
      Fix it by allocating a temporary string with dfid->path content instead
      of overwriting it in-place, which is limited in glibc snprintf with the
      __restrict qualifier.
      Reviewed-by: Andre Przywara's avatarAndre Przywara <andre.przywara@arm.com>
      Signed-off-by: default avatarAnisse Astier <aastier@freebox.fr>
      Signed-off-by: default avatarWill Deacon <will.deacon@arm.com>
      04d604b6
    • Anisse Astier's avatar
      virtio: fix warning on strncpy · 16509081
      Anisse Astier authored
      
      
      GCC 8.2 gives this warning:
      
      virtio/net.c: In function ‘virtio_net__tap_init’:
      virtio/net.c:336:47: error: argument to ‘sizeof’ in ‘strncpy’ call is the same expression as the source; did you mean to use the size of the destination? [-Werror=sizeof-pointer-memaccess]
         strncpy(ifr.ifr_name, ndev->tap_name, sizeof(ndev->tap_name));
                                                     ^
      virtio/net.c:348:47: error: argument to ‘sizeof’ in ‘strncpy’ call is the same expression as the source; did you mean to use the size of the destination? [-Werror=sizeof-pointer-memaccess]
         strncpy(ifr.ifr_name, ndev->tap_name, sizeof(ndev->tap_name));
                                                     ^
      
      Fix it by using sizeof of destination instead, even if they're the same
      size in this case.
      Reviewed-by: Andre Przywara's avatarAndre Przywara <andre.przywara@arm.com>
      Signed-off-by: default avatarAnisse Astier <aastier@freebox.fr>
      Signed-off-by: default avatarWill Deacon <will.deacon@arm.com>
      16509081
    • Anisse Astier's avatar
      builtin-run: Fix warning when resolving path · 96eda741
      Anisse Astier authored
      
      
      GCC 8.2 gives this warning:
      
      builtin-run.c: In function ‘kvm_run_write_sandbox_cmd.isra.1’:
      builtin-run.c:417:28: error: ‘%s’ directive output may be truncated writing up to 4095 bytes into a region of size 4091 [-Werror=format-truncation=]
         snprintf(dst, len, "/host%s", resolved_path);
                                  ^~   ~~~~~~~~~~~~~
      
      It's because it understands that len is PATH_MAX, the same as
      resolved_path's size. This patch handles the case where the string is
      truncated, and fixes the warning.
      Reviewed-by: Andre Przywara's avatarAndre Przywara <andre.przywara@arm.com>
      Signed-off-by: default avatarAnisse Astier <aastier@freebox.fr>
      Signed-off-by: default avatarWill Deacon <will.deacon@arm.com>
      96eda741
  2. 01 Feb, 2019 3 commits
  3. 30 Jan, 2019 2 commits
  4. 22 Jan, 2019 20 commits
  5. 02 Nov, 2018 3 commits
    • Julien Thierry's avatar
      kvm-cpu: Pause vCPU in signal handler · fdd26ecb
      Julien Thierry authored
      
      
      Currently, the handling a pause signal only sets a state that will be
      checked at the begining of the CPU run loop. At the checking point the vCPU
      sends the notification that it is actually paused allowing the pause
      requester to confirm all vCPUs are paused.
      
      Receiving the pause signal during a KVM_RUN ioctl will make KVM exit to
      userspace. However, there is a small window between that check on
      cpu->paused and the execution of KVM_RUN where the signal has been received
      but the vCPU does not go back through the notification and starts KVM_RUN.
      Since there is no guarantee the vCPU will come back to userspace, the
      pause requester might deadlock.
      
      Perform the pause directly from the signal handler. This relies on a vCPU
      thread never receiving a pause signal while being pause, but such scenario
      would have caused a deadlock for the pause requester anyway.
      Signed-off-by: default avatarJulien Thierry <julien.thierry@arm.com>
      Signed-off-by: default avatarWill Deacon <will.deacon@arm.com>
      fdd26ecb
    • Julien Thierry's avatar
      kvm: Do not pause already paused vcpus · 29f4ec31
      Julien Thierry authored
      
      
      With the following sequence:
      	kvm__pause();
      	kvm__continue();
      	kvm__pause();
      
      There is a chance that not all paused threads have been resumed, and the
      second kvm__pause will attempt to pause them again. Since the paused thread
      is waiting to own the pause_lock, it won't write its second pause
      notification. kvm__pause will be waiting for that notification while owning
      pause_lock, so... deadlock.
      
      Simple solution is not to try to pause thread that had not the chance to
      resume.
      Signed-off-by: default avatarJulien Thierry <julien.thierry@arm.com>
      Signed-off-by: default avatarWill Deacon <will.deacon@arm.com>
      29f4ec31
    • Jean-Philippe Brucker's avatar
      virtio: Fix ordering of virt_queue__available() · 66ba0bae
      Jean-Philippe Brucker authored
      
      
      After adding buffers to the virtio queue, the guest increments the avail
      index. It then reads the event index to check if it needs to notify the
      host. If the event index corresponds to the previous avail value, then
      the guest notifies the host. Otherwise it means that the host is still
      processing the queue and hasn't had a chance to increment the event
      index yet. Once it gets there, the host will see the new avail index and
      process the descriptors, so there is no need for a notification.
      
      This is only guaranteed to work if both threads write and read the
      indices in the right order. Currently a barrier is missing from
      virt_queue__available(), and the host may not see an up-to-date value of
      event index after writing avail.
      
                   HOST            |           GUEST
                                   |
                                   |    write avail = 1
                                   |    mb()
                                   |    read event -> 0
              write event = 0      |      == prev_avail -> notify
              read avail -> 1      |
                                   |
              write event = 1      |
              read avail -> 1      |
              wait()               |    write avail = 2
                                   |    mb()
                                   |    read event -> 0
                                   |      != prev_avail -> no notification
      
      By adding a memory barrier on the host side, we ensure that it doesn't
      miss any notification.
      Reviewed-By: Steven Price's avatarSteven Price <steven.price@arm.com>
      Signed-off-by: default avatarJean-Philippe Brucker <jean-philippe.brucker@arm.com>
      Signed-off-by: default avatarWill Deacon <will.deacon@arm.com>
      66ba0bae
  6. 16 Aug, 2018 1 commit
  7. 13 Jul, 2018 2 commits
  8. 06 Jul, 2018 1 commit
    • Jean-Philippe Brucker's avatar
      Fix subfolder dependency generation · 665f1b72
      Jean-Philippe Brucker authored
      
      
      When building an object "foo.o", kvmtool also creates a ".foo.o.d" file,
      using the dependency generation feature of CPP. This file describes in
      Makefile format all headers included by foo.c. When one header is
      modified, make rebuilds all objects that include it.
      
      Dependency files in subfolders are currently ignored by make, because
      the target doesn't contain the right prefix. For example virtio/.blk.o.d
      has target "blk.o" instead of "virtio/blk.o". As a result, rebuilding
      kvmtool without first issuing a make clean can introduce sneaky bugs,
      where different objects use mismatched headers. To write the right
      targets in dependency files, add a -MT argument to CPP.
      Signed-off-by: default avatarJean-Philippe Brucker <jean-philippe.brucker@arm.com>
      Signed-off-by: default avatarWill Deacon <will.deacon@arm.com>
      665f1b72