Re: [RFC PATCH v7 07/24] ceph: add fscrypt_* handling to caps.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jeff,

There has some following patches in your "fscrypt" branch, which is not posted yet, the commit is:

"3161d2f549db ceph: size handling for encrypted inodes in cap updates"

It seems buggy.

In the encode_cap_msg() you have removed the 'fscrypt_file_len' and and added a new 8 bytes' data encoding:

        ceph_encode_32(&p, arg->fscrypt_auth_len);
        ceph_encode_copy(&p, arg->fscrypt_auth, arg->fscrypt_auth_len);
-       ceph_encode_32(&p, arg->fscrypt_file_len);
-       ceph_encode_copy(&p, arg->fscrypt_file, arg->fscrypt_file_len);
+       ceph_encode_32(&p, sizeof(__le64));
+       ceph_encode_64(&p, fc->size);

That means no matter the 'arg->encrypted' is true or not, here it will always encode extra 8 bytes' data ?


But in cap_msg_size(), you are making it optional:


 static inline int cap_msg_size(struct cap_msg_args *arg)
 {
        return CAP_MSG_FIXED_FIELDS + arg->fscrypt_auth_len +
-                       arg->fscrypt_file_len;
+                       arg->encrypted ? sizeof(__le64) : 0;
 }


Have I missed something important here ?

Thanks


On 6/25/21 9:58 PM, Jeff Layton wrote:
Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
---
  fs/ceph/caps.c | 62 +++++++++++++++++++++++++++++++++++++++-----------
  1 file changed, 49 insertions(+), 13 deletions(-)

diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 038f59cc4250..1be6c5148700 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -13,6 +13,7 @@
  #include "super.h"
  #include "mds_client.h"
  #include "cache.h"
+#include "crypto.h"
  #include <linux/ceph/decode.h>
  #include <linux/ceph/messenger.h>
@@ -1229,15 +1230,12 @@ struct cap_msg_args {
  	umode_t			mode;
  	bool			inline_data;
  	bool			wake;
+	u32			fscrypt_auth_len;
+	u32			fscrypt_file_len;
+	u8			fscrypt_auth[sizeof(struct ceph_fscrypt_auth)]; // for context
+	u8			fscrypt_file[sizeof(u64)]; // for size
  };
-/*
- * cap struct size + flock buffer size + inline version + inline data size +
- * osd_epoch_barrier + oldest_flush_tid
- */
-#define CAP_MSG_SIZE (sizeof(struct ceph_mds_caps) + \
-		      4 + 8 + 4 + 4 + 8 + 4 + 4 + 4 + 8 + 8 + 4)
-
  /* Marshal up the cap msg to the MDS */
  static void encode_cap_msg(struct ceph_msg *msg, struct cap_msg_args *arg)
  {
@@ -1253,7 +1251,7 @@ static void encode_cap_msg(struct ceph_msg *msg, struct cap_msg_args *arg)
  	     arg->size, arg->max_size, arg->xattr_version,
  	     arg->xattr_buf ? (int)arg->xattr_buf->vec.iov_len : 0);
- msg->hdr.version = cpu_to_le16(10);
+	msg->hdr.version = cpu_to_le16(12);
  	msg->hdr.tid = cpu_to_le64(arg->flush_tid);
fc = msg->front.iov_base;
@@ -1324,6 +1322,16 @@ static void encode_cap_msg(struct ceph_msg *msg, struct cap_msg_args *arg)
/* Advisory flags (version 10) */
  	ceph_encode_32(&p, arg->flags);
+
+	/* dirstats (version 11) - these are r/o on the client */
+	ceph_encode_64(&p, 0);
+	ceph_encode_64(&p, 0);
+
+	/* fscrypt_auth and fscrypt_file (version 12) */
+	ceph_encode_32(&p, arg->fscrypt_auth_len);
+	ceph_encode_copy(&p, arg->fscrypt_auth, arg->fscrypt_auth_len);
+	ceph_encode_32(&p, arg->fscrypt_file_len);
+	ceph_encode_copy(&p, arg->fscrypt_file, arg->fscrypt_file_len);
  }
/*
@@ -1445,6 +1453,26 @@ static void __prep_cap(struct cap_msg_args *arg, struct ceph_cap *cap,
  		}
  	}
  	arg->flags = flags;
+	if (ci->fscrypt_auth_len &&
+	    WARN_ON_ONCE(ci->fscrypt_auth_len != sizeof(struct ceph_fscrypt_auth))) {
+		/* Don't set this if it isn't right size */
+		arg->fscrypt_auth_len = 0;
+	} else {
+		arg->fscrypt_auth_len = ci->fscrypt_auth_len;
+		memcpy(arg->fscrypt_auth, ci->fscrypt_auth,
+			min_t(size_t, ci->fscrypt_auth_len, sizeof(arg->fscrypt_auth)));
+	}
+	/* FIXME: use this to track "real" size */
+	arg->fscrypt_file_len = 0;
+}
+
+#define CAP_MSG_FIXED_FIELDS (sizeof(struct ceph_mds_caps) + \
+		      4 + 8 + 4 + 4 + 8 + 4 + 4 + 4 + 8 + 8 + 4 + 8 + 8 + 4 + 4)
+
+static inline int cap_msg_size(struct cap_msg_args *arg)
+{
+	return CAP_MSG_FIXED_FIELDS + arg->fscrypt_auth_len +
+			arg->fscrypt_file_len;
  }
/*
@@ -1457,7 +1485,7 @@ static void __send_cap(struct cap_msg_args *arg, struct ceph_inode_info *ci)
  	struct ceph_msg *msg;
  	struct inode *inode = &ci->vfs_inode;
- msg = ceph_msg_new(CEPH_MSG_CLIENT_CAPS, CAP_MSG_SIZE, GFP_NOFS, false);
+	msg = ceph_msg_new(CEPH_MSG_CLIENT_CAPS, cap_msg_size(arg), GFP_NOFS, false);
  	if (!msg) {
  		pr_err("error allocating cap msg: ino (%llx.%llx) flushing %s tid %llu, requeuing cap.\n",
  		       ceph_vinop(inode), ceph_cap_string(arg->dirty),
@@ -1483,10 +1511,6 @@ static inline int __send_flush_snap(struct inode *inode,
  	struct cap_msg_args	arg;
  	struct ceph_msg		*msg;
- msg = ceph_msg_new(CEPH_MSG_CLIENT_CAPS, CAP_MSG_SIZE, GFP_NOFS, false);
-	if (!msg)
-		return -ENOMEM;
-
  	arg.session = session;
  	arg.ino = ceph_vino(inode).ino;
  	arg.cid = 0;
@@ -1524,6 +1548,18 @@ static inline int __send_flush_snap(struct inode *inode,
  	arg.flags = 0;
  	arg.wake = false;
+ /*
+	 * No fscrypt_auth changes from a capsnap. It will need
+	 * to update fscrypt_file on size changes (TODO).
+	 */
+	arg.fscrypt_auth_len = 0;
+	arg.fscrypt_file_len = 0;
+
+	msg = ceph_msg_new(CEPH_MSG_CLIENT_CAPS, cap_msg_size(&arg),
+			   GFP_NOFS, false);
+	if (!msg)
+		return -ENOMEM;
+
  	encode_cap_msg(msg, &arg);
  	ceph_con_send(&arg.session->s_con, msg);
  	return 0;




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux