Merge pull request #67 from google/sync

security: Sync filesystem before dropping caches

4 files changed, 27 insertions, 18 deletions

diff --git a/cmd/fscrypt/commands.go b/cmd/fscrypt/commands.go
index bd4827b..2f23a0f 100644
--- a/cmd/fscrypt/commands.go
+++ b/cmd/fscrypt/commands.go
@@ -321,15 +321,16 @@ var Purge = cli.Command{
 		there are four important things to note about this command:
 
 		(1) When run with the default options, this command also clears
-		the dentry and inode cache, so that the encrypted files and
-		directories will no longer be visible. However, this requires
-		root privileges.
+		the reclaimable dentries and inodes, so that the encrypted files
+		and directories will no longer be visible. However, this
+		requires root privileges. Note that any open file descriptors to
+		plaintext data will not be affected by this command.
 
 		(2) When run with %[2]s=false, the keyring is cleared and root
 		permissions are not required, but recently accessed encrypted
 		directories and files will remain cached for some time. Because
-		of this, after purging a filesystem's keys, it is recommended to
-		unmount the filesystem.
+		of this, after purging a filesystem's keys in this manner, it
+		is recommended to unmount the filesystem.
 
 		(3) When run as root, this command removes the policy keys for
 		all users. However, this will only work if the PAM module has
@@ -382,10 +383,10 @@ func purgeAction(c *cli.Context) error {
 	fmt.Fprintf(c.App.Writer, "Policies purged for %q.\n", ctx.Mount.Path)
 
 	if dropCachesFlag.Value {
-		if err = security.DropInodeCache(); err != nil {
+		if err = security.DropFilesystemCache(); err != nil {
 			return newExitError(c, err)
 		}
-		fmt.Fprintf(c.App.Writer, "Global inode cache cleared.\n")
+		fmt.Fprintf(c.App.Writer, "Encrypted data removed filesystem cache.\n")
 	} else {
 		fmt.Fprintf(c.App.Writer, "Filesystem %q should now be unmounted.\n", ctx.Mount.Path)
 	}
diff --git a/cmd/fscrypt/flags.go b/cmd/fscrypt/flags.go
index bb8de3d..5137eff 100644
--- a/cmd/fscrypt/flags.go
+++ b/cmd/fscrypt/flags.go
@@ -164,9 +164,9 @@ var (
 	dropCachesFlag = &boolFlag{
 		Name: "drop-caches",
 		Usage: `After purging the keys from the keyring, drop the
-			inode and dentry cache for the purge to take effect.
-			Without this flag, cached encrypted files may still have
-			their plaintext visible. Requires root privileges.`,
+			associated caches for the purge to take effect. Without
+			this flag, cached encrypted files may still have their
+			plaintext visible. Requires root privileges.`,
 		Default: true,
 	}
 )
diff --git a/pam_fscrypt/pam_fscrypt.go b/pam_fscrypt/pam_fscrypt.go
index 7eccc85..571a42b 100644
--- a/pam_fscrypt/pam_fscrypt.go
+++ b/pam_fscrypt/pam_fscrypt.go
@@ -167,8 +167,8 @@ func CloseSession(handle *pam.Handle, args map[string]bool) error {
 	}
 
 	if args[cacheFlag] {
-		log.Print("dropping inode caches at session close")
-		errCache = security.DropInodeCache()
+		log.Print("dropping appropriate filesystem caches at session close")
+		errCache = security.DropFilesystemCache()
 	}
 
 	if errLock != nil {
diff --git a/security/cache.go b/security/cache.go
index 7002014..d0c60b1 100644
--- a/security/cache.go
+++ b/security/cache.go
@@ -22,20 +22,28 @@ package security
 import (
 	"log"
 	"os"
+
+	"golang.org/x/sys/unix"
 )
 
-// DropInodeCache instructs the kernel to clear the global cache of inodes and
-// dentries. This has the effect of making encrypted directories whose keys
-// are not present no longer accessible. Requires root privileges.
-func DropInodeCache() error {
-	log.Print("dropping page caches")
+// DropFilesystemCache instructs the kernel to free the reclaimable inodes and
+// dentries. This has the effect of making encrypted directories whose keys are
+// not present no longer accessible. Requires root privileges.
+func DropFilesystemCache() error {
+	// Dirty reclaimible inodes must be synced so that they will be freed.
+	log.Print("syncing changes to filesystem")
+	unix.Sync()
+
 	// See: https://www.kernel.org/doc/Documentation/sysctl/vm.txt
+	log.Print("freeing reclaimable inodes and dentries")
 	file, err := os.OpenFile("/proc/sys/vm/drop_caches", os.O_WRONLY|os.O_SYNC, 0)
 	if err != nil {
 		return err
 	}
 	defer file.Close()
-	// "2" just clears the inodes and dentries
+	// "2" just frees the reclaimable inodes and dentries, the associated
+	// pages to these inodes will be freed. We do not need to free the
+	// entire pagecache (as this will severly impact performance).
 	_, err = file.WriteString("2")
 	return err
 }