Hi tech@,

I'm looking at this code starting at line 390 of usr.sbin/vmd/config.c:

        n = virtio_get_base(diskfds[i][j], base, sizeof(base),
            vmc->vmc_disktypes[i], path);
        if (n == 0)
                break;
        if (n == -1) {
                log_warnx("vm \"%s\" unable to read "
                    "base %s for disk %s", vcp->vcp_name,
                    base, vcp->vcp_disks[i]);
                goto fail;
        }

The array "base" which is passed to log_warnx might be uninitialized:
virtio_get_base doesn't necessarily touch it if it returns -1. Maybe it
would be better just omit base from the output, e.g.

        log_warnx("vm \"%s\" unable to read "
            "base for disk %s", vcp->vcp_name,
            vcp->vcp_disks[i]);


Below are my attempts to check whether it could actually result in junk
from memory being printed...


****

I put this in /etc/vm.conf:

vm "pft" {
  owner falsifian
  disable
  memory 4G
  disk "/home/falsifian/vm/pft/disk.qcow2" format qcow2
  local interface
}

and made /home/falsifian/vm/pft/disk.qcow2 an empty file. Then when I run
"vmctl start -c pft", vmd logs the following string:

  vm "pft" unable to read base  for disk /home/falsifian/vm/pft/disk.qcow2

Okay, just an empty string; that's not too bad. To force the issue, I
added the line

  strlcpy(base, "UNINITIALIZED", sizeof base);

to the start of the config_setvm function in config.c (just after
variable declarations). Then the message became

  vm "pft" unable to read base UNINITIALIZED for disk 
/home/falsifian/vm/pft/disk.qcow2

So it seems there's at least a potential problem... C doesn't
auto-initialize stack variables, right?

To push it further, I tried to influence the log message indirectly by
inserting a function call that puts junk on the stack before the call
to config_setvm, but I couldn't get that to work. I don't know why. LMK
if you want details.

-- 
James

Reply via email to