Re: [PATCH v2 3/9] mm/mshare: make msharefs writable and support directories

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

 



On 6/30/22 15:34, Darrick J. Wong wrote:
On Wed, Jun 29, 2022 at 04:53:54PM -0600, Khalid Aziz wrote:
Make msharefs filesystem writable and allow creating directories
to support better access control to mshare'd regions defined in
msharefs.

Signed-off-by: Khalid Aziz <khalid.aziz@xxxxxxxxxx>
---
  mm/mshare.c | 195 +++++++++++++++++++++++++++++++++++++++++++++++++---
  1 file changed, 186 insertions(+), 9 deletions(-)

diff --git a/mm/mshare.c b/mm/mshare.c
index 3e448e11c742..2d5924d39221 100644
--- a/mm/mshare.c
+++ b/mm/mshare.c
@@ -21,11 +21,21 @@
  #include <linux/fileattr.h>
  #include <uapi/linux/magic.h>
  #include <uapi/linux/limits.h>
+#include <uapi/linux/mman.h>
static struct super_block *msharefs_sb; +static const struct inode_operations msharefs_dir_inode_ops;
+static const struct inode_operations msharefs_file_inode_ops;
+
+static int
+msharefs_open(struct inode *inode, struct file *file)
+{
+	return simple_open(inode, file);
+}
+
  static const struct file_operations msharefs_file_operations = {
-	.open		= simple_open,
+	.open		= msharefs_open,
  	.llseek		= no_llseek,
  };
@@ -42,6 +52,113 @@ msharefs_d_hash(const struct dentry *dentry, struct qstr *qstr)
  	return 0;
  }
+static struct dentry
+*msharefs_alloc_dentry(struct dentry *parent, const char *name)
+{
+	struct dentry *d;
+	struct qstr q;
+	int err;
+
+	q.name = name;
+	q.len = strlen(name);
+
+	err = msharefs_d_hash(parent, &q);
+	if (err)
+		return ERR_PTR(err);
+
+	d = d_alloc(parent, &q);
+	if (d)
+		return d;
+
+	return ERR_PTR(-ENOMEM);
+}
+
+static struct inode
+*msharefs_get_inode(struct super_block *sb, const struct inode *dir,
+			umode_t mode)
+{
+	struct inode *inode = new_inode(sb);
+
+	if (inode) {

Not sure why you wouldn't go with the less-indently version:

	if (!inode)
		return ERR_PTR(-ENOMEM);

	inode->i_ino = get_next_ino();
	<etc>


Yeah, good idea. I will change it.

+		inode->i_ino = get_next_ino();
+		inode_init_owner(&init_user_ns, inode, dir, mode);
+
+		inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
+
+		switch (mode & S_IFMT) {

Shouldn't we set the mode somewhere?

mode is passed in as parameter to msharefs_get_inode() which uses this value to determine its actions.


+		case S_IFREG:
+			inode->i_op = &msharefs_file_inode_ops;
+			inode->i_fop = &msharefs_file_operations;
+			break;
+		case S_IFDIR:
+			inode->i_op = &msharefs_dir_inode_ops;
+			inode->i_fop = &simple_dir_operations;
+			inc_nlink(inode);
+			break;
+		case S_IFLNK:
+			inode->i_op = &page_symlink_inode_operations;
+			break;
+		default:
+			discard_new_inode(inode);
+			inode = NULL;
+			break;
+		}
+	}
+
+	return inode;
+}
+
+static int
+msharefs_mknod(struct user_namespace *mnt_userns, struct inode *dir,
+		struct dentry *dentry, umode_t mode, dev_t dev)
+{
+	struct inode *inode;
+	int err = 0;
+
+	inode = msharefs_get_inode(dir->i_sb, dir, mode);
+	if (IS_ERR(inode))
+		return PTR_ERR(inode);

...and if @inode is NULL?

Oh right, IS_ERR() does not check for NULL value. I will add a check for that and return ENOMEM.


+
+	d_instantiate(dentry, inode);
+	dget(dentry);
+	dir->i_mtime = dir->i_ctime = current_time(dir);
+
+	return err;
+}
+
+static int
+msharefs_create(struct user_namespace *mnt_userns, struct inode *dir,
+		struct dentry *dentry, umode_t mode, bool excl)
+{
+	return msharefs_mknod(&init_user_ns, dir, dentry, mode | S_IFREG, 0);
+}
+
+static int
+msharefs_mkdir(struct user_namespace *mnt_userns, struct inode *dir,
+		struct dentry *dentry, umode_t mode)
+{
+	int ret = msharefs_mknod(&init_user_ns, dir, dentry, mode | S_IFDIR, 0);
+
+	if (!ret)
+		inc_nlink(dir);
+	return ret;
+}
+
+static const struct inode_operations msharefs_file_inode_ops = {
+	.setattr	= simple_setattr,
+	.getattr	= simple_getattr,
+};
+static const struct inode_operations msharefs_dir_inode_ops = {
+	.create		= msharefs_create,
+	.lookup		= simple_lookup,
+	.link		= simple_link,
+	.unlink		= simple_unlink,
+	.mkdir		= msharefs_mkdir,
+	.rmdir		= simple_rmdir,
+	.mknod		= msharefs_mknod,
+	.rename		= simple_rename,
+};
+
  static void
  mshare_evict_inode(struct inode *inode)
  {
@@ -58,7 +175,7 @@ mshare_info_read(struct file *file, char __user *buf, size_t nbytes,
  {
  	char s[80];
- sprintf(s, "%ld", PGDIR_SIZE);
+	sprintf(s, "%ld\n", PGDIR_SIZE);

Changing this already?

Possibly. There is one suggestion to change it to PMD and it might be a better choice.


  	return simple_read_from_buffer(buf, nbytes, ppos, s, strlen(s));
  }
@@ -72,6 +189,38 @@ static const struct super_operations mshare_s_ops = {
  	.evict_inode = mshare_evict_inode,
  };
+static int
+prepopulate_files(struct super_block *s, struct inode *dir,
+			struct dentry *root, const struct tree_descr *files)
+{
+	int i;
+	struct inode *inode;
+	struct dentry *dentry;
+
+	for (i = 0; !files->name || files->name[0]; i++, files++) {
+		if (!files->name)
+			continue;

What ends the array?  NULL name or empty name?
Do we have to erase all of these when the fs gets unmounted?

This code is very similar to simple_fill_super() and I reused the code from there. inodes and dentries will need to be erased on unmount through evict_inode.

Thanks,
Khalid



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

  Powered by Linux