zfs crypto discuss - Apr 2008 - [Bug 1159] New: ''zfs key -u'' hangs after ''zfs set keysource''

http://defect.opensolaris.org/bz/show_bug.cgi?id=1159

           Summary: ''zfs key -u'' hangs after ''zfs set
keysource''
    Classification: Development
           Product: zfs-crypto
           Version: unspecified
          Platform: Other
        OS/Version: Solaris
            Status: NEW
          Severity: major
          Priority: P2
         Component: other
        AssignedTo: darrenm at opensolaris.org
        ReportedBy: hua.tang at sun.com
         QAContact: hua.tang at sun.com
                CC: zfs-crypto-discuss at opensolaris.org
   Estimated Hours: 0.0


Build: 2008-04-10

Below are steps:
# zpool create pool_101616 /var/tmp/zfs-crypto/vdev_file
# zfs create -o encryption=on -o keyscope=dataset -o
keysource=raw,file:///export/home/zfscrypto-tests/proto/suites/security/zfs-crypto/etc/raw_key_file
pool_101616/fs_1
# zfs set
keysource=raw,file:///export/home/zfscrypto-tests/proto/suites/security/zfs-crypto/etc/new_raw_key_file
pool_101616/fs_1
# cp
/export/home/zfscrypto-tests/proto/suites/security/zfs-crypto/etc/file1.txt 
/pool_101616/fs_1/
# zfs key -u pool_101616/fs_1
^C


It hangs in kernel because Ctrl+c isn''t able to interrupt it.


Note: the two raw key files have different contents.

-- 
Configure bugmail: http://defect.opensolaris.org/bz/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

http://defect.opensolaris.org/bz/show_bug.cgi?id=1159


Darren J Moffat <darrenm at opensolaris.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ACCEPTED




--- Comment #1 from Darren J Moffat <darrenm at opensolaris.org> 
2008-05-07 09:02:57 ---
The ''zfs set keysource'' doesn''t appear to be relevant
I reproduced this without
that step.

-- 
Configure bugmail: http://defect.opensolaris.org/bz/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

http://defect.opensolaris.org/bz/show_bug.cgi?id=1159


Darren J Moffat <darrenm at opensolaris.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ACCEPTED                    |CLOSED
         Resolution|                            |FIXINSOURCE




--- Comment #2 from Darren J Moffat <darrenm at opensolaris.org> 
2008-05-08 02:35:42 ---
Based on my experiementation to reproduce this it appears that because we
unload the key before we unmount the filesystem there could be data that
hasn''t
yet been  written out to disk.

Given that the fix for this bug would be to unmount the dataset first:

--- a/usr/src/lib/libzfs/common/libzfs_crypto.c Thu May 08 09:59:20 2008 +0100
+++ b/usr/src/lib/libzfs/common/libzfs_crypto.c Thu May 08 10:21:34 2008 +0100
@@ -315,6 +315,25 @@ zfs_unload_key(zfs_handle_t *zhp)
                    "Improper keyscope for this command."));
                return (EINVAL);
        }
+
+       /*
+        * We need to be sure that all the data has been written to
+        * disk before we unload the key so we first have to attempt
+        * an unmount, if that fails we don''t continue with the key
unload
+        * and instead return the error from zfs_umount.
+        * Maybe in the future ''zfs key'' will grow a -f flag
in which case
+        * MS_FORCE would get passed to zfs_umount, but for now that flag
+        * doesn''t exist.
+        */
+       if (zfs_is_mounted(zhp, &mntpt))
+               ret = zfs_unmount(zhp, mntpt, 0);
+               if (ret) {
+                       int err = errno;
+                       zfs_error_aux(zhp->zfs_hdl, dgettext(TEXT_DOMAIN,
+                               "Failed to unload key: unmount
failed"));
+                       return (ret);
+               }
+
        zc.zc_crypto.zic_cmd = ZFS_IOC_CRYPTO_UNLOAD_KEY_DSL;
        (void) strlcpy(zc.zc_name, zfs_get_name(zhp), sizeof (zc.zc_name));

@@ -327,15 +346,6 @@ zfs_unload_key(zfs_handle_t *zhp)
        }

        zfs_refresh_properties(zhp);
-
-       /*
-        * We don''t care if there are complications with unmounting
this
-        * the dataset.  If it cannot be umounted because there are
-        * mounted datasets underneath it, then that is fine.  The key
-        * is unloaded and the data is unaccessable
-        */
-       if (zfs_is_mounted(zhp, &mntpt))
-               (void) zfs_unmount(zhp, mntpt, NULL);

        return (0);
 }


Note that this is a change in what we had agreed the design would be but it
seems like the only safe thing to do.  Doing the umount causes the data to be
written out to disk by this happening in zfsvfs_teardown()

        if (dmu_objset_evict_dbufs(zfsvfs->z_os)) {
                txg_wait_synced(dmu_objset_pool(zfsvfs->z_os), 0);
                (void) dmu_objset_evict_dbufs(zfsvfs->z_os);
        }

-- 
Configure bugmail: http://defect.opensolaris.org/bz/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.