Change error handling to new package

This commit changes the error handing for the crypto, filesystem,
metadata, pam, and util packages to use the error handling library
github.com/pkg/errors. This means elimination of the FSError type, an
increased use of wrapping errors (as opposed to logging), switching
on the Cause() of an error (as opposed to its value), and improving our
integration tests involving TEST_FILESYSTEM_ROOT.

This commit also fixes a few bugs with the keyring code to ensure that
our {Find|Remove|Insert}PolicyKey functions are always operating on the
same keyring. The check for filesystem support has been moved from the
filesystem package to the metadata package. Finally, the API for the
filesystem package has been slightly modified:
	* filesystem.AllFilesystems() now returns all the filesystems in
	  sorted order
	* certain path methods are now public

O_SYNC is also removed for writing the metadata. We don't get that much
from syncing the metadata, as the actual file data could also be
corrupted by and IO error. The sync operation is also occasionally very
slow (~3 seconds) and can be unfriendly to battery life.

Change-Id: I392c2655141714b16dfdbc84ac09780072be2cf0

5 files changed, 113 insertions, 153 deletions

diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go
index 434826b..960c06f 100644
--- a/filesystem/filesystem.go
+++ b/filesystem/filesystem.go
@@ -33,7 +33,6 @@
 package filesystem
 
 import (
-	"errors"
 	"fmt"
 	"io/ioutil"
 	"log"
@@ -42,41 +41,26 @@ import (
 	"strings"
 
 	"github.com/golang/protobuf/proto"
+	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
 
 	"fscrypt/metadata"
 	"fscrypt/util"
 )
 
-// FSError is the error type returned by all Mount methods. It contains an
-// error value as well as the corresponding filesystem path. The error value
-// is generally one of the errors defined in this package or an underlying
-// error from the operating system.
-type FSError struct {
-	Path string
-	Err  error
-}
-
-func (m FSError) Error() string {
-	return fmt.Sprintf("filesystem %q: %v", m.Path, m.Err)
-}
-
 // Filesystem error values
 var (
-	ErrBadLoad         = util.SystemError("couldn't load mountpoint info")
-	ErrRootNotMount    = util.SystemError("reached root directory without finding a mountpoint")
-	ErrInvalidMount    = errors.New("invalid mountpoint provided")
-	ErrNotSetup        = errors.New("not setup for use with fscrypt")
+	ErrNotAMountpoint  = errors.New("not a mountpoint")
 	ErrAlreadySetup    = errors.New("already setup for use with fscrypt")
-	ErrBadState        = util.SystemError("metadata directory in bad state: rerun setup")
+	ErrNotSetup        = errors.New("not setup for use with fscrypt")
+	ErrNoMetadata      = errors.New("could not find metadata")
+	ErrLinkedProtector = errors.New("not a regular protector")
 	ErrInvalidMetadata = errors.New("provided metadata is invalid")
-	ErrCorruptMetadata = util.SystemError("metadata is corrupt")
-	ErrNoMetadata      = errors.New("no metadata could be found for the provided descriptor")
-	ErrLinkedProtector = errors.New("descriptor corresponds to a linked protector")
-	ErrCannotLink      = util.SystemError("cannot create filesystem link")
-	ErrNoLink          = util.SystemError("link does not point to a valid filesystem")
-	ErrOldLink         = util.SystemError("link points to filesystems not using fscrypt")
-	ErrNoSupport       = errors.New("this filesystem does not support encryption")
+	ErrFollowLink      = errors.New("cannot follow filesystem link")
+	ErrLinkExpired     = errors.New("no longer exists on linked filesystem")
+	ErrMakeLink        = util.SystemError("cannot create filesystem link")
+	ErrGlobalMountInfo = util.SystemError("creating global mountpoint list failed")
+	ErrCorruptMetadata = util.SystemError("on-disk metadata is corrupt")
 )
 
 // Mount contains information for a specific mounted filesystem.
@@ -138,24 +122,24 @@ const (
 func (m *Mount) String() string {
 	return fmt.Sprintf(`%s
 	Filsystem: %s
-	Options:    %v
-	Device:      %s`, m.Path, m.Filesystem, m.Options, m.Device)
+	Options:   %v
+	Device:    %s`, m.Path, m.Filesystem, m.Options, m.Device)
 }
 
-// baseDir returns the path of the base fscrypt directory on this filesystem.
-func (m *Mount) baseDir() string {
+// BaseDir returns the path of the base fscrypt directory on this filesystem.
+func (m *Mount) BaseDir() string {
 	return filepath.Join(m.Path, baseDirName)
 }
 
-// protectorDir returns the directory containing the protector metadata.
-func (m *Mount) protectorDir() string {
-	return filepath.Join(m.baseDir(), protectorDirName)
+// ProtectorDir returns the directory containing the protector metadata.
+func (m *Mount) ProtectorDir() string {
+	return filepath.Join(m.BaseDir(), protectorDirName)
 }
 
 // protectorPath returns the full path to a regular protector file with the
 // specified descriptor.
 func (m *Mount) protectorPath(descriptor string) string {
-	return filepath.Join(m.protectorDir(), descriptor)
+	return filepath.Join(m.ProtectorDir(), descriptor)
 }
 
 // linkedProtectorPath returns the full path to a linked protector file with the
@@ -164,15 +148,15 @@ func (m *Mount) linkedProtectorPath(descriptor string) string {
 	return m.protectorPath(descriptor) + linkFileExtension
 }
 
-// policyDir returns the directory containing the policy metadata.
-func (m *Mount) policyDir() string {
-	return filepath.Join(m.baseDir(), policyDirName)
+// PolicyDir returns the directory containing the policy metadata.
+func (m *Mount) PolicyDir() string {
+	return filepath.Join(m.BaseDir(), policyDirName)
 }
 
 // policyPath returns the full path to a regular policy file with the
 // specified descriptor.
 func (m *Mount) policyPath(descriptor string) string {
-	return filepath.Join(m.policyDir(), descriptor)
+	return filepath.Join(m.PolicyDir(), descriptor)
 }
 
 // tempMount creates a temporary Mount under the main directory. The path for
@@ -182,28 +166,22 @@ func (m *Mount) tempMount() (*Mount, error) {
 	return &Mount{Path: trashDir}, err
 }
 
-// err creates a FSErr for this filesystem with the provided error. If the
-// passed error is an OS error, the full error is logged, but only the
-// underlying error is used in the message. If the message is nil, nil is
-// returned.
+// err modifies an error to contain the path of this filesystem.
 func (m *Mount) err(err error) error {
-	if err == nil {
-		return nil
-	}
-
-	return FSError{
-		Path: m.Path,
-		Err:  util.UnderlyingError(err),
-	}
+	return errors.Wrapf(err, "filesystem %s", m.Path)
 }
 
-// CheckSetup returns an error if all the fscrypt metadata directories exist.
-// Will log any unexpected errors, or if any permissions are incorrect.
+// CheckSetup returns an error if this filesystem does not support fscrypt or
+// all the fscrypt metadata directories do not exist. Will log any unexpected
+// errors or incorrect permissions.
 func (m *Mount) CheckSetup() error {
+	if err := metadata.CheckSupport(m.Path); err != nil {
+		return m.err(err)
+	}
 	// Run all the checks so we will always get all the warnings
-	baseGood := isDirCheckPerm(m.baseDir(), basePermissions)
-	policyGood := isDirCheckPerm(m.policyDir(), dirPermissions)
-	protectorGood := isDirCheckPerm(m.protectorDir(), dirPermissions)
+	baseGood := isDirCheckPerm(m.BaseDir(), basePermissions)
+	policyGood := isDirCheckPerm(m.PolicyDir(), dirPermissions)
+	protectorGood := isDirCheckPerm(m.ProtectorDir(), dirPermissions)
 
 	if baseGood && policyGood && protectorGood {
 		return nil
@@ -220,13 +198,13 @@ func (m *Mount) makeDirectories() error {
 		unix.Umask(oldMask)
 	}()
 
-	if err := os.Mkdir(m.baseDir(), basePermissions); err != nil {
+	if err := os.Mkdir(m.BaseDir(), basePermissions); err != nil {
 		return err
 	}
-	if err := os.Mkdir(m.policyDir(), dirPermissions); err != nil {
+	if err := os.Mkdir(m.PolicyDir(), dirPermissions); err != nil {
 		return err
 	}
-	return os.Mkdir(m.protectorDir(), dirPermissions)
+	return os.Mkdir(m.ProtectorDir(), dirPermissions)
 }
 
 // Setup sets up the filesystem for use with fscrypt, note that this merely
@@ -234,8 +212,13 @@ func (m *Mount) makeDirectories() error {
 // the filesystem's feature flags. This operation is atomic, it either succeeds
 // or no files in the baseDir are created.
 func (m *Mount) Setup() error {
-	if m.CheckSetup() == nil {
+	switch err := m.CheckSetup(); errors.Cause(err) {
+	case ErrNotSetup:
+		break
+	case nil:
 		return m.err(ErrAlreadySetup)
+	default:
+		return err
 	}
 	// We build the directories under a temp Mount and then move into place.
 	temp, err := m.tempMount()
@@ -248,13 +231,8 @@ func (m *Mount) Setup() error {
 		return m.err(err)
 	}
 
-	// Move directory into place. If the base directory exists despite our
-	// earlier check that we were not setup, we are in bad state.
-	err = os.Rename(temp.baseDir(), m.baseDir())
-	if os.IsExist(err) {
-		err = ErrBadState
-	}
-	return m.err(err)
+	// Atomically move directory into place.
+	return m.err(os.Rename(temp.BaseDir(), m.BaseDir()))
 }
 
 // RemoveAllMetadata removes all the policy and protector metadata from the
@@ -274,7 +252,7 @@ func (m *Mount) RemoveAllMetadata() error {
 	defer os.RemoveAll(temp.Path)
 
 	// Move directory into temp (to be destroyed on defer)
-	return m.err(os.Rename(m.baseDir(), temp.baseDir()))
+	return m.err(os.Rename(m.BaseDir(), temp.BaseDir()))
 }
 
 // writeDataAtomic writes the data to the path such that the data is either
@@ -283,8 +261,7 @@ func (m *Mount) writeDataAtomic(path string, data []byte) error {
 	// Write the file to a temporary file then move into place so that the
 	// operation will be atomic.
 	tempPath := filepath.Join(filepath.Dir(path), tempPrefix+filepath.Base(path))
-	// We use O_SYNC so the write actually gets to stable storage.
-	tempFile, err := os.OpenFile(tempPath, os.O_WRONLY|os.O_CREATE|os.O_SYNC, filePermissions)
+	tempFile, err := os.OpenFile(tempPath, os.O_WRONLY|os.O_CREATE, filePermissions)
 	if err != nil {
 		return err
 	}
@@ -304,8 +281,8 @@ func (m *Mount) writeDataAtomic(path string, data []byte) error {
 // addMetadata writes the metadata structure to the file with the specified
 // path this will overwrite any existing data. The operation is atomic.
 func (m *Mount) addMetadata(path string, md metadata.Metadata) error {
-	if !md.IsValid() {
-		return ErrInvalidMetadata
+	if err := md.CheckValidity(); err != nil {
+		return errors.Wrap(ErrInvalidMetadata, err.Error())
 	}
 
 	data, err := proto.Marshal(md)
@@ -322,20 +299,20 @@ func (m *Mount) addMetadata(path string, md metadata.Metadata) error {
 func (m *Mount) getMetadata(path string, md metadata.Metadata) error {
 	data, err := ioutil.ReadFile(path)
 	if err != nil {
+		log.Printf("could not read metadata at %q", path)
 		if os.IsNotExist(err) {
-			return ErrNoMetadata
+			return errors.Wrapf(ErrNoMetadata, "descriptor %s", filepath.Base(path))
 		}
 		return err
 	}
 
-	if err = proto.Unmarshal(data, md); err != nil {
-		log.Print(err)
-		return ErrCorruptMetadata
+	if err := proto.Unmarshal(data, md); err != nil {
+		return errors.Wrap(ErrCorruptMetadata, err.Error())
 	}
 
-	if !md.IsValid() {
-		log.Printf("data retrieved at %q is not valid", path)
-		return ErrCorruptMetadata
+	if err := md.CheckValidity(); err != nil {
+		log.Printf("metadata at %q is not valid", path)
+		return errors.Wrap(ErrCorruptMetadata, err.Error())
 	}
 
 	log.Printf("successfully read metadata from %q", path)
@@ -346,8 +323,9 @@ func (m *Mount) getMetadata(path string, md metadata.Metadata) error {
 // path. Works with regular or linked metadata.
 func (m *Mount) removeMetadata(path string) error {
 	if err := os.Remove(path); err != nil {
+		log.Printf("could not remove metadata at %q", path)
 		if os.IsNotExist(err) {
-			return ErrNoMetadata
+			return errors.Wrapf(ErrNoMetadata, "descriptor %s", filepath.Base(path))
 		}
 		return err
 	}
@@ -429,11 +407,13 @@ func (m *Mount) GetProtector(descriptor string) (*Mount, *metadata.ProtectorData
 	}
 
 	for _, mnt := range mnts {
-		if data, err := mnt.GetRegularProtector(descriptor); err == nil {
+		if data, err := mnt.GetRegularProtector(descriptor); err != nil {
+			log.Print(err)
+		} else {
 			return mnt, data, nil
 		}
 	}
-	return nil, nil, m.err(ErrOldLink)
+	return nil, nil, m.err(errors.Wrapf(ErrLinkExpired, "protector %s", descriptor))
 }
 
 // RemoveProtector deletes the protector metadata (or an link to another
@@ -445,7 +425,7 @@ func (m *Mount) RemoveProtector(descriptor string) error {
 	// We first try to remove the linkedProtector. If that metadata does not
 	// exist, we try to remove the normal protector.
 	err := m.removeMetadata(m.linkedProtectorPath(descriptor))
-	if err == ErrNoMetadata {
+	if errors.Cause(err) == ErrNoMetadata {
 		err = m.removeMetadata(m.protectorPath(descriptor))
 	}
 	return m.err(err)
@@ -457,7 +437,7 @@ func (m *Mount) ListProtectors() ([]string, error) {
 	if err := m.CheckSetup(); err != nil {
 		return nil, err
 	}
-	protectors, err := m.listDirectory(m.protectorDir())
+	protectors, err := m.listDirectory(m.ProtectorDir())
 	return protectors, m.err(err)
 }
 
@@ -492,7 +472,7 @@ func (m *Mount) ListPolicies() ([]string, error) {
 	if err := m.CheckSetup(); err != nil {
 		return nil, err
 	}
-	policies, err := m.listDirectory(m.policyDir())
+	policies, err := m.listDirectory(m.PolicyDir())
 	return policies, m.err(err)
 }
 
diff --git a/filesystem/filesystem_test.go b/filesystem/filesystem_test.go
index 33ab10b..bcf4f38 100644
--- a/filesystem/filesystem_test.go
+++ b/filesystem/filesystem_test.go
@@ -20,14 +20,16 @@
 package filesystem
 
 import (
-	"fmt"
 	"os"
 	"path/filepath"
 	"reflect"
 	"testing"
 
+	"github.com/pkg/errors"
+
 	. "fscrypt/crypto"
 	. "fscrypt/metadata"
+	. "fscrypt/util"
 )
 
 var (
@@ -37,17 +39,14 @@ var (
 	wrappedPolicyKey, _    = Wrap(fakeProtectorKey, fakePolicyKey)
 )
 
-// Gets the mount corresponding to TEST_FILESYSTEM_ROOT
+// Gets the mount corresponding to the integration test path.
 func getTestMount() (*Mount, error) {
-	mountpoint := os.Getenv("TEST_FILESYSTEM_ROOT")
-	if mountpoint == "" {
-		return nil, fmt.Errorf("set TEST_FILESYSTEM_ROOT to a mountpoint")
-	}
-	mnt, err := GetMount(mountpoint)
+	mountpoint, err := TestPath()
 	if err != nil {
-		return nil, fmt.Errorf("bad TEST_FILESYSTEM_ROOT: %s", err)
+		return nil, err
 	}
-	return mnt, nil
+	mnt, err := GetMount(mountpoint)
+	return mnt, errors.Wrapf(err, TestEnvVarName)
 }
 
 func getFakeProtector() *ProtectorData {
@@ -92,7 +91,7 @@ func TestSetup(t *testing.T) {
 		t.Error(err)
 	}
 
-	os.RemoveAll(mnt.baseDir())
+	os.RemoveAll(mnt.BaseDir())
 }
 
 // Tests that we can remove all of the metadata
@@ -106,7 +105,7 @@ func TestRemoveAllMetadata(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if isDir(mnt.baseDir()) {
+	if isDir(mnt.BaseDir()) {
 		t.Error("metadata was not removed")
 	}
 }
@@ -279,7 +278,7 @@ func TestLinkedProtector(t *testing.T) {
 
 	// Get the protector though the second system
 	_, err = fakeMnt.GetRegularProtector(protector.ProtectorDescriptor)
-	if err == nil || err.(FSError).Err != ErrNoMetadata {
+	if errors.Cause(err) != ErrNoMetadata {
 		t.Fatal(err)
 	}
 
diff --git a/filesystem/mountpoint.go b/filesystem/mountpoint.go
index 1a4b10f..1fc41be 100644
--- a/filesystem/mountpoint.go
+++ b/filesystem/mountpoint.go
@@ -42,10 +42,11 @@ import (
 	"fmt"
 	"log"
 	"path/filepath"
+	"sort"
 	"strings"
 	"sync"
 
-	"fscrypt/metadata"
+	"github.com/pkg/errors"
 )
 
 var (
@@ -75,7 +76,8 @@ func getMountInfo() error {
 	// Load the mount information from mountpoints_filename
 	fileHandle := C.setmntent(C.mountpoints_filename, C.read_mode)
 	if fileHandle == nil {
-		return ErrBadLoad
+		return errors.Wrapf(ErrGlobalMountInfo, "could not read %q",
+			C.GoString(C.mountpoints_filename))
 	}
 	defer C.endmntent(fileHandle)
 
@@ -84,7 +86,7 @@ func getMountInfo() error {
 		C.blkid_put_cache(cache)
 	}
 	if C.blkid_get_cache(&cache, nil) != 0 {
-		return ErrBadLoad
+		return errors.Wrap(ErrGlobalMountInfo, "could not read blkid cache")
 	}
 
 	for {
@@ -105,11 +107,12 @@ func getMountInfo() error {
 		// Skip invalid mountpoints
 		var err error
 		if mnt.Path, err = cannonicalizePath(mnt.Path); err != nil {
-			log.Print(err)
+			log.Printf("getting mnt_dir: %v", err)
 			continue
 		}
 		// We can only use mountpoints that are directories for fscrypt.
 		if !isDir(mnt.Path) {
+			log.Printf("mnt_dir %v: not a directory", mnt.Path)
 			continue
 		}
 
@@ -127,41 +130,22 @@ func getMountInfo() error {
 	}
 }
 
-// checkSupport returns an error if the specified mount does not support
-// filesystem-level encryption.
-func checkSupport(mount *Mount) error {
-	// Getting a policy on a filesystem which supports encryption should
-	// either return the policy or say there isn't one. Anything else
-	// indicates a problem with support.
-	_, err := metadata.GetPolicy(mount.Path)
-	if err == nil || err == metadata.ErrNotEncrypted {
-		log.Printf("%s filesystem at %q supports encryption (got %v)",
-			mount.Filesystem, mount.Path, err)
-		return nil
-	}
-
-	log.Printf("%s filesystem at %q probably doesn't support encryption (got %v)",
-		mount.Filesystem, mount.Path, err)
-	return err
-}
-
-// AllSupportedFilesystems lists all the Mounts which could support filesystem
-// encryption. This doesn't mean they necessarily do or that they are being used
-// with fscrypt.
-func AllSupportedFilesystems() ([]*Mount, error) {
+// AllFilesystems lists all the Mounts on the current system ordered by path.
+// Use CheckSetup() to see if they are used with fscrypt.
+func AllFilesystems() ([]*Mount, error) {
 	mountMutex.Lock()
 	defer mountMutex.Unlock()
 	if err := getMountInfo(); err != nil {
 		return nil, err
 	}
 
-	var supportedMounts []*Mount
-	for _, mount := range mountsByPath {
-		if checkSupport(mount) == nil {
-			supportedMounts = append(supportedMounts, mount)
-		}
+	mounts := make([]*Mount, len(mountsByPath))
+	for i, mount := range mountsByPath {
+		mounts[i] = mount
 	}
-	return supportedMounts, nil
+
+	sort.Sort(PathSorter(mounts))
+	return mounts, nil
 }
 
 // UpdateMountInfo updates the filesystem mountpoint maps with the current state
@@ -176,9 +160,9 @@ func UpdateMountInfo() error {
 // FindMount returns the corresponding Mount object for some path in a
 // filesystem. Note that in the case of a bind mounts there may be two Mount
 // objects for the same underlying filesystem. An error is returned if the path
-// is invalid, we cannot load the required mount data, or the filesystem does
-// not support filesystem encryption. If a filesystem has been updated since the
-// last call to one of the mount functions, run UpdateMountInfo to see changes.
+// is invalid or we cannot load the required mount data. If a filesystem has
+// been updated since the last call to one of the mount functions, run
+// UpdateMountInfo to see changes.
 func FindMount(path string) (*Mount, error) {
 	path, err := cannonicalizePath(path)
 	if err != nil {
@@ -194,23 +178,22 @@ func FindMount(path string) (*Mount, error) {
 	// Traverse up the directory tree until we find a mountpoint
 	for {
 		if mnt, ok := mountsByPath[path]; ok {
-			return mnt, checkSupport(mnt)
+			return mnt, nil
 		}
 
 		// Move to the parent directory unless we have reached the root.
 		parent := filepath.Dir(path)
 		if parent == path {
-			return nil, ErrRootNotMount
+			return nil, errors.Wrap(ErrNotAMountpoint, path)
 		}
 		path = parent
 	}
 }
 
 // GetMount returns the Mount object with a matching mountpoint. An error is
-// returned if the path is invalid, we cannot load the required mount data, or
-// the filesystem does not support filesystem encryption. If a filesystem has
-// been updated since the last call to one of the mount functions, run
-// UpdateMountInfo to see changes.
+// returned if the path is invalid or we cannot load the required mount data. If
+// a filesystem has been updated since the last call to one of the mount
+// functions, run UpdateMountInfo to see changes.
 func GetMount(mountpoint string) (*Mount, error) {
 	mountpoint, err := cannonicalizePath(mountpoint)
 	if err != nil {
@@ -224,11 +207,10 @@ func GetMount(mountpoint string) (*Mount, error) {
 	}
 
 	if mnt, ok := mountsByPath[mountpoint]; ok {
-		return mnt, checkSupport(mnt)
+		return mnt, nil
 	}
 
-	log.Printf("%q is not a filesystem mountpoint", mountpoint)
-	return nil, ErrInvalidMount
+	return nil, errors.Wrap(ErrNotAMountpoint, mountpoint)
 }
 
 // getMountsFromLink returns the Mount objects which match the provided link.
@@ -251,7 +233,7 @@ func getMountsFromLink(link string) ([]*Mount, error) {
 	log.Printf("blkid_evaluate_spec(%q, <cache>) = %q", link, deviceName)
 
 	if deviceName == "" {
-		return nil, ErrNoLink
+		return nil, errors.Wrapf(ErrFollowLink, "link %q is invalid", link)
 	}
 	deviceName, err := cannonicalizePath(deviceName)
 	if err != nil {
@@ -268,7 +250,7 @@ func getMountsFromLink(link string) ([]*Mount, error) {
 		return mnts, nil
 	}
 
-	return nil, ErrNoLink
+	return nil, errors.Wrapf(ErrFollowLink, "device %q is invalid", deviceName)
 }
 
 // makeLink returns a link of the form <token>=<value> where value is the tag
@@ -297,7 +279,7 @@ func makeLink(mnt *Mount, token string) (string, error) {
 	log.Printf("blkid_get_tag_value(<cache>, %s, %s) = %s", token, deviceEntry, value)
 
 	if value == "" {
-		return "", ErrCannotLink
+		return "", errors.Wrapf(ErrMakeLink, "no %s", token)
 	}
 	return fmt.Sprintf("%s=%s", token, C.GoString(cValue)), nil
 }
diff --git a/filesystem/mountpoint_test.go b/filesystem/mountpoint_test.go
index 5523451..5d53dc1 100644
--- a/filesystem/mountpoint_test.go
+++ b/filesystem/mountpoint_test.go
@@ -39,14 +39,6 @@ func printMountInfo() {
 	}
 }
 
-func printSupportedMounts() {
-	fmt.Println("\nSupported Mountpoints:")
-	mnts, _ := AllSupportedFilesystems()
-	for _, mnt := range mnts {
-		fmt.Println(mnt)
-	}
-}
-
 func TestLoadMountInfo(t *testing.T) {
 	if err := UpdateMountInfo(); err != nil {
 		t.Error(err)
@@ -56,7 +48,6 @@ func TestLoadMountInfo(t *testing.T) {
 func TestPrintMountInfo(t *testing.T) {
 	// Uncomment to see the mount info in the tests
 	// printMountInfo()
-	// printSupportedMounts()
 	// t.Fail()
 }
 
diff --git a/filesystem/path.go b/filesystem/path.go
index 3be1859..d788a6b 100644
--- a/filesystem/path.go
+++ b/filesystem/path.go
@@ -23,6 +23,8 @@ import (
 	"log"
 	"os"
 	"path/filepath"
+
+	"github.com/pkg/errors"
 )
 
 // We only check the unix permissions and the sticky bit
@@ -34,8 +36,14 @@ func cannonicalizePath(path string) (string, error) {
 	if err != nil {
 		return "", err
 	}
+	path, err = filepath.EvalSymlinks(path)
+
+	// Get a better error if we have an invalid path
+	if pathErr, ok := err.(*os.PathError); ok {
+		err = errors.Wrap(pathErr.Err, pathErr.Path)
+	}
 
-	return filepath.EvalSymlinks(path)
+	return path, err
 }
 
 // loggedStat runs os.Stat, but it logs the error if stat returns any error