patch 9.0.1669: Crash syncing swapfile in new buffer when using sodium crypt

Problem:    Crash syncing swapfile in new buffer when using sodium crypt.
            (James McCoy)
Solution:   Add checks for sodium encryption. (Christian Brabandt,
            closes #12591, closes #12585)

Author: chrisbra

src/crypt.c

@@ -1267,6 +1267,13 @@ crypt_sodium_buffer_decode(
 }
 
 # if defined(FEAT_SODIUM) || defined(PROTO)
+    void
+crypt_sodium_lock_key(char_u *key)
+{
+    if (sodium_init() >= 0)
+	sodium_mlock(key, STRLEN(key));
+}
+
     int
 crypt_sodium_munlock(void *const addr, const size_t len)
 {

src/memline.c

@@ -424,6 +424,24 @@ ml_open(buf_T *buf)
 }
 
 #if defined(FEAT_CRYPT) || defined(PROTO)
+/*
+ * Swapfile encryption is not supported by XChaCha20.  If this crypt method is
+ * used then disable the swapfile, to avoid plain text being written to disk,
+ * and return TRUE.
+ * Otherwise return FALSE.
+ */
+    static int
+crypt_may_close_swapfile(buf_T *buf, char_u *key, int method)
+{
+    if (crypt_method_is_sodium(method) && *key != NUL)
+    {
+	mf_close_file(buf, TRUE);
+	buf->b_p_swf = FALSE;
+	return TRUE;
+    }
+    return FALSE;
+}
+
 /*
  * Prepare encryption for "buf" for the current key and method.
  */
@@ -440,11 +458,10 @@ ml_set_mfp_crypt(buf_T *buf)
 	// Generate a seed and store it in the memfile.
 	sha2_seed(buf->b_ml.ml_mfp->mf_seed, MF_SEED_LEN, NULL, 0);
     }
-#ifdef FEAT_SODIUM
+# ifdef FEAT_SODIUM
     else if (crypt_method_is_sodium(method_nr))
-	crypt_sodium_randombytes_buf(buf->b_ml.ml_mfp->mf_seed,
-		MF_SEED_LEN);
-#endif
+	crypt_sodium_randombytes_buf(buf->b_ml.ml_mfp->mf_seed, MF_SEED_LEN);
+# endif
 }
 
 /*
@@ -501,16 +518,10 @@ ml_set_crypt_key(
 	return;  // no memfile yet, nothing to do
     old_method = crypt_method_nr_from_name(old_cm);
 
-    // Swapfile encryption is not supported by XChaCha20, therefore disable the
-    // swapfile to avoid plain text being written to disk.
-    if (crypt_method_is_sodium(crypt_get_method_nr(buf))
-						       && *buf->b_p_key != NUL)
-    {
-	// close the swapfile
-	mf_close_file(buf, TRUE);
-	buf->b_p_swf = FALSE;
+#ifdef FEAT_CRYPT
+    if (crypt_may_close_swapfile(buf, buf->b_p_key, crypt_get_method_nr(buf)))
 	return;
-    }
+#endif
 
     // First make sure the swapfile is in a consistent state, using the old
     // key and method.
@@ -2494,6 +2505,12 @@ ml_sync_all(int check_file, int check_char)
 		|| buf->b_ml.ml_mfp->mf_fd < 0)
 	    continue;			    // no file
 
+#ifdef FEAT_CRYPT
+	if (crypt_may_close_swapfile(buf, buf->b_p_key,
+						     crypt_get_method_nr(buf)))
+	    continue;
+#endif
+
 	ml_flush_line(buf);		    // flush buffered line
 					    // flush locked block
 	(void)ml_find_line(buf, (linenr_T)0, ML_FLUSH);
@@ -2551,6 +2568,10 @@ ml_preserve(buf_T *buf, int message)
 	    emsg(_(e_cannot_preserve_there_is_no_swap_file));
 	return;
     }
+#ifdef FEAT_CRYPT
+    if (crypt_may_close_swapfile(buf, buf->b_p_key, crypt_get_method_nr(buf)))
+	return;
+#endif
 
     // We only want to stop when interrupted here, not when interrupted
     // before.
@@ -5571,6 +5592,9 @@ ml_crypt_prepare(memfile_T *mfp, off_T offset, int reading)
     if (*key == NUL)
 	return NULL;
 
+    if (crypt_may_close_swapfile(buf, key, method_nr))
+	return NULL;
+
     if (method_nr == CRYPT_M_ZIP)
     {
 	// For PKzip: Append the offset to the key, so that we use a different

src/optionstr.c

@@ -1174,6 +1174,10 @@ did_set_cryptkey(optset_T *args)
 		*curbuf->b_p_cm == NUL ? p_cm : curbuf->b_p_cm);
 	changed_internal();
     }
+# ifdef FEAT_SODIUM
+    if (crypt_method_is_sodium(crypt_get_method_nr(curbuf)))
+       crypt_sodium_lock_key(args->os_newval.string);
+# endif
 
     return NULL;
 }

src/proto/crypt.pro

@@ -26,6 +26,7 @@ void crypt_check_swapfile_curbuf(void);
 void crypt_check_current_method(void);
 char_u *crypt_get_key(int store, int twice);
 void crypt_append_msg(buf_T *buf);
+void crypt_sodium_lock_key(char_u *key);
 int crypt_sodium_munlock(void *const addr, const size_t len);
 void crypt_sodium_randombytes_buf(void *const buf, const size_t size);
 int crypt_sodium_init(void);

src/testdir/test_crypt.vim

@@ -105,7 +105,7 @@ func Test_crypt_sodium_v2_startup()
   exe buf .. 'bwipe!'
   call assert_true(filereadable('Xfoo'))
 
-  let buf = RunVimInTerminal('--cmd "set ch=3 cm=xchacha20v2 key=foo" Xfoo', #{rows: 10})
+  let buf = RunVimInTerminal('--cmd "set ch=3 cm=xchacha20v2 key=foo" Xfoo', #{wait_for_ruler: 0, rows: 10})
   call g:TermWait(buf, g:RunningWithValgrind() ? 1000 : 50)
   call StopVimInTerminal(buf)
 
@@ -392,4 +392,24 @@ func Test_crypt_set_key_changes_buffer()
   call delete('Xtest1.txt')
 endfunc
 
+func Test_crypt_set_key_segfault()
+  CheckFeature sodium
+
+  defer delete('Xtest2.txt')
+  new Xtest2.txt
+  call setline(1, 'nothing')
+  set cryptmethod=xchacha20
+  set key=foobar
+  w
+  new Xtest3
+  put ='other content'
+  setl modified
+  sil! preserve
+  bwipe!
+
+  set cryptmethod&
+  set key=
+  bwipe!
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab

src/version.c

@@ -695,6 +695,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1669,
 /**/
     1668,
 /**/