Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] vcsim: createVM crashes when VM name contains / #2873

Closed
Syuparn opened this issue Jun 10, 2022 · 2 comments · Fixed by #2874
Closed

[BUG] vcsim: createVM crashes when VM name contains / #2873

Syuparn opened this issue Jun 10, 2022 · 2 comments · Fixed by #2874
Assignees

Comments

@Syuparn
Copy link
Contributor

Syuparn commented Jun 10, 2022

Describe the bug

CreateVM request crashes vcsim when the vm name contains /.

To Reproduce

  1. start vcsim without options
  2. execute govc command below
$ govc vm.create -cluster DC0_C0 -net DC0_DVPG0 "foo/bar"

Expected behavior

A new vm named foo%2fbar is created. Special character / is escaped automatically.

$ govc vm.create -cluster DC0_C0 -net DC0_DVPG0 "foo/bar"
$ govc vm.info "foo%2fbar"
Name:           foo%2fbar
  Path:         /DC0/vm/foo%2fbar
  UUID:         4221b558-d433-d07d-af97-b56a71f990c1
  Guest name:   Other (32-bit)
  Memory:       1024MB
  CPU:          1 vCPU(s)
  Power state:  poweredOn
  Boot time:    2022-06-10 12:57:15 +0000 UTC
  IP address:
  Host:         192.168.1.200

Affected version

  • vcsim v0.28.0

Screenshots/Debug Output
If applicable, add screenshots or debug output to help explain your problem.

vcsim raises panic due to nil pointer dereference while creating the VM.

$ govc vm.create -cluster DC0_C0 -net DC0_DVPG0 "foo/bar"
govc: Post "https://127.0.0.1:8989/sdk": read tcp 127.0.0.1:40060->127.0.0.1:8989: read: connection reset by peer
$ vcsim
export GOVC_URL=https://user:pass@127.0.0.1:8989/sdk GOVC_SIM_PID=228005
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xac4977]

goroutine 103 [running]:
github.com/vmware/govmomi/simulator.(*VirtualMachine).Reference(0x0?)
        <autogenerated>:1 +0x17
github.com/vmware/govmomi/simulator.folderRemoveChild(0xc0008c01b0, 0xc000364b60, {0x110c420, 0x0})
        /home/runner/work/govmomi/govmomi/simulator/folder.go:112 +0x38
github.com/vmware/govmomi/simulator.(*createVM).Run(0xc0002c96a0, 0x0?)
        /home/runner/work/govmomi/govmomi/simulator/folder.go:372 +0x555
github.com/vmware/govmomi/simulator.(*Task).Run.func1()
        /home/runner/work/govmomi/govmomi/simulator/task.go:117 +0x6c
created by github.com/vmware/govmomi/simulator.(*Task).Run
        /home/runner/work/govmomi/govmomi/simulator/task.go:115 +0x238

Additional context

This seems due to unescaped special characters (like #2717). %, /, and \ in VM names should be escaped like folder names.

# In vCenter
$ govc vm.create -net DC0_DVPG0 "/\%"
$ govc vm.info "%2f%5c%25"
Name:           %2f%5c%25
  Path:         /DC0/vm/%2f%5c%25
  UUID:         4221d450-2e38-eadf-dd45-29b6a13a9b49
  Guest name:   Other (32-bit)
  Memory:       1024MB
  CPU:          1 vCPU(s)
  Power state:  poweredOn
  Boot time:    2022-06-10 13:13:34 +0000 UTC
  IP address:
  Host:         192.168.1.200
@embano1 embano1 self-assigned this Jun 10, 2022
@Syuparn
Copy link
Contributor Author

Syuparn commented Jun 11, 2022

Nil pointer dereference occurs in folderRemoveChild because a vm is not put in the folder when NewVirtualMachine returns an error.

https://github.com/vmware/govmomi/blob/master/simulator/folder.go#L372

https://github.com/vmware/govmomi/blob/master/simulator/virtual_machine.go#L170

folderRemoveChild in L:372 seems unnecessary.

@embano1
Copy link
Contributor

embano1 commented Jun 11, 2022

Thx, wanna file a PR?

Syuparn added a commit to Syuparn/govmomi that referenced this issue Jun 12, 2022
Closes: vmware#2873
Signed-off-by: syuparn <s.hello.spagetti@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants