Re: [PATCH v1 05/17] btrfs: add inode encryption contexts

Sweet Tea Dorminy <sweettea-kernel@xxxxxxxxxx> · Sun, 16 Jul 2023 21:43:39 -0400

On 7/7/23 19:32, Boris Burkov wrote:

On Wed, Jun 28, 2023 at 08:35:28PM -0400, Sweet Tea Dorminy wrote:

From: Omar Sandoval <osandov@xxxxxxxxxxx>

In order to store encryption information for directories, symlinks,
etc., fscrypt stores a context item with each encrypted non-regular
inode. fscrypt provides an arbitrary blob for the filesystem to store,
and it does not clearly fit into an existing structure, so this goes in
a new item type.

Signed-off-by: Omar Sandoval <osandov@xxxxxxxxxxx>
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@xxxxxxxxxx>
---
  fs/btrfs/fscrypt.c              | 116 ++++++++++++++++++++++++++++++++
  fs/btrfs/fscrypt.h              |   2 +
  fs/btrfs/inode.c                |  19 ++++++
  fs/btrfs/ioctl.c                |   8 ++-
  include/uapi/linux/btrfs_tree.h |  10 +++
  5 files changed, 153 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/fscrypt.c b/fs/btrfs/fscrypt.c
index 3a53dc59c1e4..235f65e43d96 100644
--- a/fs/btrfs/fscrypt.c
+++ b/fs/btrfs/fscrypt.c
@@ -1,8 +1,124 @@
  // SPDX-License-Identifier: GPL-2.0

  

+#include <linux/iversion.h>

  #include "ctree.h"
+#include "accessors.h"
+#include "btrfs_inode.h"
+#include "disk-io.h"
+#include "fs.h"
  #include "fscrypt.h"
+#include "ioctl.h"
+#include "messages.h"
+#include "transaction.h"
+#include "xattr.h"
+
+static int btrfs_fscrypt_get_context(struct inode *inode, void *ctx, size_t len)
+{
+	struct btrfs_key key = {
+		.objectid = btrfs_ino(BTRFS_I(inode)),
+		.type = BTRFS_FSCRYPT_CTXT_ITEM_KEY,
+		.offset = 0,
+	};
+	struct btrfs_path *path;
+	struct extent_buffer *leaf;
+	unsigned long ptr;
+	int ret;
+
+
+	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
+
+	ret = btrfs_search_slot(NULL, BTRFS_I(inode)->root, &key, path, 0, 0);
+	if (ret) {
+		len = -EINVAL;


I'm a little wary about squishing the errors down like this. It could
be some error, in which case it might be interesting to get the real errno
or it could be ret > 1, in which case I think ENOENT is more useful than
EINVAL.


I'll make it ENOENT.


Also, having a ret variable and mashing that into len feels kinda weird.
Maybe that's the neatest way to write this logic, though.



It's the way the existing fscrypt interface does things, so it'd be hard 

to change.




Since the variables usually go by ctx, I lightly prefer CTX_ITEM_KEY.
Obviously not a big deal.

Sure, will do.


