The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6315
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
- Fixes LXC snapshot rename crash
- Replaces ExtractSnapshotName with ContainerGetParentAndSnapshotName
From 9b3a8998daa65d79d5aa261dfa3983856d48112b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 17 Oct 2019 10:30:34 +0100
Subject: [PATCH 1/3] lxc/storage/volume: Fix panic when invalid snapshot
rename argument supplied
When running
lxc storage volume rename test myvol6/snap0 myvol18
I got this crash:
panic: runtime error: index out of range
goroutine 1 [running]:
github.com/lxc/lxd/shared.ExtractSnapshotName(0x7fff387181b1, 0x7, 0xb512d6,
0x1)
/home/user/go/src/github.com/lxc/lxd/shared/util.go:487 +0x84
main.(*cmdStorageVolumeRename).Run(0xc42022ad60, 0xc4202a4f00, 0xc420288ae0,
0x3, 0x3, 0x0, 0x0)
/home/user/go/src/github.com/lxc/lxd/lxc/storage_volume.go:1217 +0x285
main.(*cmdStorageVolumeRename).Run-fm(0xc4202a4f00, 0xc420288ae0, 0x3, 0x3,
0x0, 0x0)
/home/user/go/src/github.com/lxc/lxd/lxc/storage_volume.go:1177 +0x52
github.com/spf13/cobra.(*Command).execute(0xc4202a4f00, 0xc420288a20, 0x3, 0x3,
0xc4202a4f00, 0xc420288a20)
/home/user/go/src/github.com/spf13/cobra/command.go:829 +0x468
github.com/spf13/cobra.(*Command).ExecuteC(0xc420208280, 0x0, 0xc420296d80,
0x19)
/home/user/go/src/github.com/spf13/cobra/command.go:917 +0x306
github.com/spf13/cobra.(*Command).Execute(0xc420208280, 0xc4200aa010, 0x6)
/home/user/go/src/github.com/spf13/cobra/command.go:867 +0x2b
main.main()
/home/user/go/src/github.com/lxc/lxd/lxc/main.go:238 +0x1de8
This is because the rename command is expecting the 2nd volume name in the
format:
lxc storage volume rename test myvol6/snap0 myvol6/newname
This fixes this issue, as using ExtractSnapshotName is unsafe if you've not
checked whether the volume name is a snapshot or not.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxc/storage_volume.go | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/lxc/storage_volume.go b/lxc/storage_volume.go
index 111ae58df2..9bdc70de5e 100644
--- a/lxc/storage_volume.go
+++ b/lxc/storage_volume.go
@@ -1214,7 +1214,13 @@ func (c *cmdStorageVolumeRename) Run(cmd *cobra.Command,
args []string) error {
if isSnapshot {
// Create the storage volume entry
vol := api.StorageVolumeSnapshotPost{}
- vol.Name = shared.ExtractSnapshotName(args[2])
+ _, dstSnapName, dstIsSnap :=
shared.ContainerGetParentAndSnapshotName(args[2])
+
+ if !dstIsSnap {
+ return fmt.Errorf("Invalid new snapshot name")
+ }
+
+ vol.Name = dstSnapName
// If a target member was specified, get the volume with the
matching
// name on that member, if any.
From c5e766b0a35dbd34ce513c34b0f8c0a85374754b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 17 Oct 2019 10:32:42 +0100
Subject: [PATCH 2/3] shared/util: Removes ExtractSnapshotName
ExtractSnapshotName was dangerous to use as it did not validate the lenth of
the slice returned when splitting the supplied volume name by the snapshot
delimiter (/).
When used with untrusted inputs it could cause a crash.
Additionally it's functionality overlapped with the safer and more feature rich
ContainerGetParentAndSnapshotName, so have removed in place of that.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
shared/util.go | 4 ----
1 file changed, 4 deletions(-)
diff --git a/shared/util.go b/shared/util.go
index 4bcfc30bbc..e307f985fe 100644
--- a/shared/util.go
+++ b/shared/util.go
@@ -483,10 +483,6 @@ func IsSnapshot(name string) bool {
return strings.Contains(name, SnapshotDelimiter)
}
-func ExtractSnapshotName(name string) string {
- return strings.SplitN(name, SnapshotDelimiter, 2)[1]
-}
-
func MkdirAllOwner(path string, perm os.FileMode, uid int, gid int) error {
// This function is a slightly modified version of MkdirAll from the Go
standard library.
// https://golang.org/src/os/path.go?s=488:535#L9
From ff178f7a248900ca41da6e7752e11a439c1debaf Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 17 Oct 2019 10:36:18 +0100
Subject: [PATCH 3/3] lxd: Changes use of ExtractSnapshotName to
ContainerGetParentAndSnapshotName
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/migrate_container.go | 3 ++-
lxd/migrate_storage_volumes.go | 5 +++--
lxd/storage_volumes_utils.go | 3 ++-
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/lxd/migrate_container.go b/lxd/migrate_container.go
index 1a0c094556..69b5d362ff 100644
--- a/lxd/migrate_container.go
+++ b/lxd/migrate_container.go
@@ -386,7 +386,8 @@ func (s *migrationSourceWs) Do(migrateOp
*operations.Operation) error {
if err == nil {
for _, snap := range fullSnaps {
snapshots = append(snapshots,
snapshotToProtobuf(snap))
- snapshotNames = append(snapshotNames,
shared.ExtractSnapshotName(snap.Name()))
+ _, snapName, _ :=
shared.ContainerGetParentAndSnapshotName(snap.Name())
+ snapshotNames = append(snapshotNames, snapName)
}
}
}
diff --git a/lxd/migrate_storage_volumes.go b/lxd/migrate_storage_volumes.go
index 05ba8e7186..1c492dedb2 100644
--- a/lxd/migrate_storage_volumes.go
+++ b/lxd/migrate_storage_volumes.go
@@ -69,7 +69,8 @@ func (s *migrationSourceWs) DoStorage(migrateOp
*operations.Operation) error {
}
snapshots = append(snapshots,
volumeSnapshotToProtobuf(snapVolume))
- snapshotNames = append(snapshotNames,
shared.ExtractSnapshotName(name))
+ _, snapName, _ :=
shared.ContainerGetParentAndSnapshotName(name)
+ snapshotNames = append(snapshotNames,
snapName)
}
}
@@ -429,7 +430,7 @@ func volumeSnapshotToProtobuf(vol *api.StorageVolume)
*migration.Snapshot {
config = append(config, &migration.Config{Key: &kCopy, Value:
&vCopy})
}
- snapOnlyName := shared.ExtractSnapshotName(vol.Name)
+ _, snapOnlyName, _ := shared.ContainerGetParentAndSnapshotName(vol.Name)
return &migration.Snapshot{
Name: &snapOnlyName,
diff --git a/lxd/storage_volumes_utils.go b/lxd/storage_volumes_utils.go
index 46eeb2fac8..aa02763399 100644
--- a/lxd/storage_volumes_utils.go
+++ b/lxd/storage_volumes_utils.go
@@ -647,7 +647,8 @@ func storagePoolVolumeCreateInternal(state *state.State,
poolName string, vol *a
}
for _, snap := range snapshots {
- _, err :=
storagePoolVolumeSnapshotCopyInternal(state, poolName, vol,
shared.ExtractSnapshotName(snap))
+ _, snapName, _ :=
shared.ContainerGetParentAndSnapshotName(snap)
+ _, err :=
storagePoolVolumeSnapshotCopyInternal(state, poolName, vol, snapName)
if err != nil {
return err
}
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
