Repository: ceph/go-ceph
Pull request #239 (2020-05-04 08:29:09)

cephfs: Validate mount.mount usage in cephfs/path.go
ceph MountInfo pointer is being passed in the functions without a validation check, if it is nil there may be a crash. Fixed the functions to first validate mount.mount

Signed-off-by: Mudit Agarwal muagarwa@redhat.com

## Checklist
- [x] Added tests for features and functional changes
- [x] Public functions and types are documented
- [x] Standard formatting is applied to Go code



Patch:
diff --git a/cephfs/cephfs.go b/cephfs/cephfs.go
index 03ce64d..288ab47 100644
--- a/cephfs/cephfs.go
+++ b/cephfs/cephfs.go
@@ -29,6 +29,14 @@ func createMount(id *C.char) (*MountInfo, error) {
 	return mount, nil
 }
 
+// validate checks whether mount.mount is ready to use or not.
+func (mount *MountInfo) validate() error {
+	if mount.mount == nil {
+		return ErrNotConnected
+	}
+	return nil
+}
+
 // CreateMount creates a mount handle for interacting with Ceph.
 func CreateMount() (*MountInfo, error) {
 	return createMount(nil)
diff --git a/cephfs/path.go b/cephfs/path.go
index 865580e..e4bd5b8 100644
--- a/cephfs/path.go
+++ b/cephfs/path.go
@@ -14,12 +14,18 @@ import (
 
 // CurrentDir gets the current working directory.
 func (mount *MountInfo) CurrentDir() string {
+	if err := mount.validate(); err != nil {
+		return ""
+	}
 	cDir := C.ceph_getcwd(mount.mount)
 	return C.GoString(cDir)
 }
 
 // ChangeDir changes the current working directory.
 func (mount *MountInfo) ChangeDir(path string) error {
+	if err := mount.validate(); err != nil {
+		return err
+	}
 	cPath := C.CString(path)
 	defer C.free(unsafe.Pointer(cPath))
 
@@ -29,6 +35,9 @@ func (mount *MountInfo) ChangeDir(path string) error {
 
 // MakeDir creates a directory.
 func (mount *MountInfo) MakeDir(path string, mode uint32) error {
+	if err := mount.validate(); err != nil {
+		return err
+	}
 	cPath := C.CString(path)
 	defer C.free(unsafe.Pointer(cPath))
 
@@ -38,6 +47,9 @@ func (mount *MountInfo) MakeDir(path string, mode uint32) error {
 
 // RemoveDir removes a directory.
 func (mount *MountInfo) RemoveDir(path string) error {
+	if err := mount.validate(); err != nil {
+		return err
+	}
 	cPath := C.CString(path)
 	defer C.free(unsafe.Pointer(cPath))
 
@@ -50,6 +62,9 @@ func (mount *MountInfo) RemoveDir(path string) error {
 // Implements:
 //  int ceph_unlink(struct ceph_mount_info *cmount, const char *path);
 func (mount *MountInfo) Unlink(path string) error {
+	if err := mount.validate(); err != nil {
+		return err
+	}
 	cPath := C.CString(path)
 	defer C.free(unsafe.Pointer(cPath))
 
@@ -62,6 +77,9 @@ func (mount *MountInfo) Unlink(path string) error {
 // Implements:
 //  int ceph_link (struct ceph_mount_info *cmount, const char *existing, const char *newname);
 func (mount *MountInfo) Link(oldname, newname string) error {
+	if err := mount.validate(); err != nil {
+		return err
+	}
 	cOldname := C.CString(oldname)
 	defer C.free(unsafe.Pointer(cOldname))
 
@@ -77,6 +95,9 @@ func (mount *MountInfo) Link(oldname, newname string) error {
 // Implements:
 //  int ceph_symlink(struct ceph_mount_info *cmount, const char *existing, const char *newname);
 func (mount *MountInfo) Symlink(existing, newname string) error {
+	if err := mount.validate(); err != nil {
+		return err
+	}
 	cExisting := C.CString(existing)
 	defer C.free(unsafe.Pointer(cExisting))
 
@@ -92,6 +113,9 @@ func (mount *MountInfo) Symlink(existing, newname string) error {
 // Implements:
 //  int ceph_readlink(struct ceph_mount_info *cmount, const char *path, char *buf, int64_t size);
 func (mount *MountInfo) Readlink(path string) (string, error) {
+	if err := mount.validate(); err != nil {
+		return "", err
+	}
 	cPath := C.CString(path)
 	defer C.free(unsafe.Pointer(cPath))
 


Test patch:
diff --git a/cephfs/cephfs_test.go b/cephfs/cephfs_test.go
index 90419e3..ddbd112 100644
--- a/cephfs/cephfs_test.go
+++ b/cephfs/cephfs_test.go
@@ -269,3 +269,57 @@ func TestGetSetConfigOption(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, origVal, currVal)
 }
+
+func TestValidate(t *testing.T) {
+	mount, err := CreateMount()
+	assert.NoError(t, err)
+	assert.NotNil(t, mount)
+	defer assert.NoError(t, mount.Release())
+
+	t.Run("mountCurrentDir", func(t *testing.T) {
+		path := mount.CurrentDir()
+		assert.Equal(t, path, "")
+	})
+
+	t.Run("mountChangeDir", func(t *testing.T) {
+		err := mount.ChangeDir("someDir")
+		assert.Error(t, err)
+		assert.Equal(t, err, ErrNotConnected)
+	})
+
+	t.Run("mountMakeDir", func(t *testing.T) {
+		err := mount.MakeDir("someName", 0444)
+		assert.Error(t, err)
+		assert.Equal(t, err, ErrNotConnected)
+	})
+
+	t.Run("mountRemoveDir", func(t *testing.T) {
+		err := mount.RemoveDir("someDir")
+		assert.Error(t, err)
+		assert.Equal(t, err, ErrNotConnected)
+	})
+
+	t.Run("mountLink", func(t *testing.T) {
+		err := mount.Link("/", "/")
+		assert.Error(t, err)
+		assert.Equal(t, err, ErrNotConnected)
+	})
+
+	t.Run("mountUnlink", func(t *testing.T) {
+		err := mount.Unlink("someFile")
+		assert.Error(t, err)
+		assert.Equal(t, err, ErrNotConnected)
+	})
+
+	t.Run("mountSymlink", func(t *testing.T) {
+		err := mount.Symlink("/", "/")
+		assert.Error(t, err)
+		assert.Equal(t, err, ErrNotConnected)
+	})
+
+	t.Run("mountReadlink", func(t *testing.T) {
+		_, err := mount.Readlink("somePath")
+		assert.Error(t, err)
+		assert.Equal(t, err, ErrNotConnected)
+	})
+}