Re: [PATCH v6 10/17] fs-verity: implement FS_IOC_ENABLE_VERITY ioctl

@@ -147,6 +147,7 @@ FS_IOC_ENABLE_VERITY can fail with the following errors:
 
 - ``EACCES``: the process does not have write access to the file
 - ``EBADMSG``: the signature is malformed
+- ``EBUSY``: this ioctl is already running on the file
 - ``EEXIST``: the file already has verity enabled
 - ``EFAULT``: the caller provided inaccessible memory
 - ``EINTR``: the operation was interrupted by a fatal signal

@@ -113,6 +113,9 @@ static int ext4_begin_enable_verity(struct file *filp)
 	handle_t *handle;
 	int err;
 
+	if (ext4_verity_in_progress(inode))
+		return -EBUSY;
+
 	/*
 	 * Since the file was opened readonly, we have to initialize the jbd
 	 * inode and quotas here and not rely on ->open() doing it.  This must

@@ -123,6 +123,9 @@ static int f2fs_begin_enable_verity(struct file *filp)
 	struct inode *inode = file_inode(filp);
 	int err;
 
+	if (f2fs_verity_in_progress(inode))
+		return -EBUSY;
+
 	if (f2fs_is_atomic_file(inode) || f2fs_is_volatile_file(inode))
 		return -EOPNOTSUPP;
 

@@ -187,8 +187,6 @@ static int enable_verity(struct file *filp,
 
 	desc->data_size = cpu_to_le64(inode->i_size);
 
-	pr_debug("Building Merkle tree...\n");
-
 	/* Prepare the Merkle tree parameters */
 	err = fsverity_init_merkle_tree_params(&params, inode,
 					       arg->hash_algorithm,

@@ -197,12 +195,29 @@ static int enable_verity(struct file *filp,
 	if (err)
 		goto out;
 
-	/* Tell the filesystem that verity is being enabled on the file */
-	err = vops->begin_enable_verity(filp);
+	/*
+	 * Start enabling verity on this file, serialized by the inode lock.
+	 * Fail if verity is already enabled or is already being enabled.
+	 */
+	inode_lock(inode);
+	if (IS_VERITY(inode))
+		err = -EEXIST;
+	else
+		err = vops->begin_enable_verity(filp);
+	inode_unlock(inode);
 	if (err)
 		goto out;
 
-	/* Build the Merkle tree */
+	/*
+	 * Build the Merkle tree.  Don't hold the inode lock during this, since
+	 * on huge files it may take a very long time and we don't want to force
+	 * unrelated syscalls like chown() to block forever.  We don't need the
+	 * inode lock because deny_write_access() already prevents the file from
+	 * being written to or truncated, and we still serialize
+	 * ->begin_enable_verity() and ->end_enable_verity() with the inode lock
+	 * and only allow one process to be here at a time.
+	 */
+	pr_debug("Building Merkle tree...\n");
 	BUILD_BUG_ON(sizeof(desc->root_hash) < FS_VERITY_MAX_DIGEST_SIZE);
 	err = build_merkle_tree(inode, &params, desc->root_hash);
 	if (err) {

@@ -229,8 +244,13 @@ static int enable_verity(struct file *filp,
 		pr_debug("Storing a %u-byte PKCS#7 signature alongside the file\n",
 			 arg->sig_size);
 
-	/* Tell the filesystem to finish enabling verity on the file */
+	/*
+	 * Tell the filesystem to finish enabling verity on the file.  The
+	 * inode_lock() serializes this with ->begin_enable_verity().
+	 */
+	inode_lock(inode);
 	err = vops->end_enable_verity(filp, desc, desc_size, params.tree_size);
+	inode_unlock(inode);
 	if (err) {
 		fsverity_err(inode, "%ps() failed with err %d",
 			     vops->end_enable_verity, err);

@@ -254,7 +274,9 @@ static int enable_verity(struct file *filp,
 	return err;
 
 rollback:
+	inode_lock(inode);
 	(void)vops->end_enable_verity(filp, NULL, 0, params.tree_size);
+	inode_unlock(inode);
 	goto out;
 }
 

@@ -319,16 +341,9 @@ int fsverity_ioctl_enable(struct file *filp, const void __user *uarg)
 	if (err) /* -ETXTBSY */
 		goto out_drop_write;
 
-	inode_lock(inode);
-
-	if (IS_VERITY(inode)) {
-		err = -EEXIST;
-		goto out_unlock;
-	}
-
 	err = enable_verity(filp, &arg);
 	if (err)
-		goto out_unlock;
+		goto out_allow_write_access;
 
 	/*
 	 * Some pages of the file may have been evicted from pagecache after

@@ -345,8 +360,7 @@ int fsverity_ioctl_enable(struct file *filp, const void __user *uarg)
 	 * allow_write_access() is needed to pair with deny_write_access().
 	 * Regardless, the filesystem won't allow writing to verity files.
 	 */
-out_unlock:
-	inode_unlock(inode);
+out_allow_write_access:
 	allow_write_access(filp);
 out_drop_write:
 	mnt_drop_write_file(filp);

@@ -23,7 +23,8 @@ struct fsverity_operations {
 	 * @filp: a readonly file descriptor for the file
 	 *
 	 * The filesystem must do any needed filesystem-specific preparations
-	 * for enabling verity, e.g. evicting inline data.
+	 * for enabling verity, e.g. evicting inline data.  It also must return
+	 * -EBUSY if verity is already being enabled on the given file.
 	 *
 	 * i_rwsem is held for write.
 	 *

@@ -46,7 +47,8 @@ struct fsverity_operations {
 	 * inode, e.g. setting a bit in the on-disk inode.  The filesystem is
 	 * also responsible for setting the S_VERITY flag in the VFS inode.
 	 *
-	 * i_rwsem is held for write.
+	 * i_rwsem is held for write, but it may have been dropped between the
+	 * calls to ->begin_enable_verity() and ->end_enable_verity().
 	 *
 	 * Return: 0 on success, -errno on failure
 	 */

@@ -96,7 +98,7 @@ struct fsverity_operations {
 	 * @log_blocksize: log base 2 of the Merkle tree block size
 	 *
 	 * This is only called between ->begin_enable_verity() and
-	 * ->end_enable_verity().  i_rwsem is held for write.
+	 * ->end_enable_verity().
 	 *
 	 * Return: 0 on success, -errno on failure
 	 */