linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Isaac J. Manjarres" <isaacmanjarres@google.com>
To: lorenzo.stoakes@oracle.com, Andrew Morton <akpm@linux-foundation.org>
Cc: kaleshsingh@google.com, jstultz@google.com, aliceryhl@google.com,
	 surenb@google.com,
	"Isaac J. Manjarres" <isaacmanjarres@google.com>,
	kernel-team@android.com,  linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH v2 1/2] mm/memfd: Refactor and cleanup the logic in memfd_create()
Date: Tue,  7 Jan 2025 10:48:01 -0800	[thread overview]
Message-ID: <20250107184804.4074147-2-isaacmanjarres@google.com> (raw)
In-Reply-To: <20250107184804.4074147-1-isaacmanjarres@google.com>

memfd_create() is a pretty busy function that could be easier to read
if some of the logic was split out into helper functions.

Therefore, split the flags check, name creation, and file creation into
their own helper functions, and create the file structure before
creating the memfd. This allows for simplifying the error handling path
in memfd_create().

No functional change.

Signed-off-by: Isaac J. Manjarres <isaacmanjarres@google.com>
---
 mm/memfd.c | 87 +++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 56 insertions(+), 31 deletions(-)

diff --git a/mm/memfd.c b/mm/memfd.c
index 5f5a23c9051d..a9430090bb20 100644
--- a/mm/memfd.c
+++ b/mm/memfd.c
@@ -369,16 +369,8 @@ int memfd_check_seals_mmap(struct file *file, unsigned long *vm_flags_ptr)
 	return err;
 }
 
-SYSCALL_DEFINE2(memfd_create,
-		const char __user *, uname,
-		unsigned int, flags)
+static int memfd_validate_flags(unsigned int flags)
 {
-	unsigned int *file_seals;
-	struct file *file;
-	int fd, error;
-	char *name;
-	long len;
-
 	if (!(flags & MFD_HUGETLB)) {
 		if (flags & ~(unsigned int)MFD_ALL_FLAGS)
 			return -EINVAL;
@@ -393,20 +385,25 @@ SYSCALL_DEFINE2(memfd_create,
 	if ((flags & MFD_EXEC) && (flags & MFD_NOEXEC_SEAL))
 		return -EINVAL;
 
-	error = check_sysctl_memfd_noexec(&flags);
-	if (error < 0)
-		return error;
+	return check_sysctl_memfd_noexec(&flags);
+}
+
+static char *memfd_create_name(const char __user *uname)
+{
+	int error;
+	char *name;
+	long len;
 
 	/* length includes terminating zero */
 	len = strnlen_user(uname, MFD_NAME_MAX_LEN + 1);
 	if (len <= 0)
-		return -EFAULT;
+		return ERR_PTR(-EFAULT);
 	if (len > MFD_NAME_MAX_LEN + 1)
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 
 	name = kmalloc(len + MFD_NAME_PREFIX_LEN, GFP_KERNEL);
 	if (!name)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	strcpy(name, MFD_NAME_PREFIX);
 	if (copy_from_user(&name[MFD_NAME_PREFIX_LEN], uname, len)) {
@@ -420,11 +417,22 @@ SYSCALL_DEFINE2(memfd_create,
 		goto err_name;
 	}
 
-	fd = get_unused_fd_flags((flags & MFD_CLOEXEC) ? O_CLOEXEC : 0);
-	if (fd < 0) {
-		error = fd;
-		goto err_name;
-	}
+	return name;
+
+err_name:
+	kfree(name);
+	return ERR_PTR(error);
+}
+
+static struct file *memfd_file_create(const char *name, unsigned int flags)
+{
+	unsigned int *file_seals;
+	struct file *file;
+	int error;
+
+	error = memfd_validate_flags(flags);
+	if (error < 0)
+		return ERR_PTR(error);
 
 	if (flags & MFD_HUGETLB) {
 		file = hugetlb_file_setup(name, 0, VM_NORESERVE,
@@ -433,10 +441,8 @@ SYSCALL_DEFINE2(memfd_create,
 					MFD_HUGE_MASK);
 	} else
 		file = shmem_file_setup(name, 0, VM_NORESERVE);
-	if (IS_ERR(file)) {
-		error = PTR_ERR(file);
-		goto err_fd;
-	}
+	if (IS_ERR(file))
+		return file;
 	file->f_mode |= FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE;
 	file->f_flags |= O_LARGEFILE;
 
@@ -456,13 +462,32 @@ SYSCALL_DEFINE2(memfd_create,
 			*file_seals &= ~F_SEAL_SEAL;
 	}
 
-	fd_install(fd, file);
-	kfree(name);
-	return fd;
+	return file;
+}
 
-err_fd:
-	put_unused_fd(fd);
-err_name:
+SYSCALL_DEFINE2(memfd_create,
+		const char __user *, uname,
+		unsigned int, flags)
+{
+	struct file *file;
+	int fd;
+	char *name;
+
+	name = memfd_create_name(uname);
+	if (IS_ERR(name))
+		return PTR_ERR(name);
+
+	file = memfd_file_create(name, flags);
+	/* name is not needed beyond this point. */
 	kfree(name);
-	return error;
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	fd = get_unused_fd_flags((flags & MFD_CLOEXEC) ? O_CLOEXEC : 0);
+	if (fd >= 0)
+		fd_install(fd, file);
+	else
+		fput(file);
+
+	return fd;
 }
-- 
2.47.1.613.gc27f4b7a9f-goog



  reply	other threads:[~2025-01-07 18:48 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-07 18:48 [PATCH v2 0/2] Cleanup for memfd_create() Isaac J. Manjarres
2025-01-07 18:48 ` Isaac J. Manjarres [this message]
2025-01-08 13:31   ` [PATCH v2 1/2] mm/memfd: Refactor and cleanup the logic in memfd_create() Alice Ryhl
2025-01-08 18:40     ` Isaac Manjarres
2025-01-08 18:30   ` Lorenzo Stoakes
2025-01-08 20:04     ` Isaac Manjarres
2025-01-08 20:23       ` Lorenzo Stoakes
2025-01-07 18:48 ` [PATCH v2 2/2] mm/memfd: Use strncpy_from_user() to read memfd name Isaac J. Manjarres
2025-01-08 13:43   ` Alice Ryhl
2025-01-08 18:43     ` Isaac Manjarres
2025-01-08 18:58   ` Lorenzo Stoakes
2025-01-09  2:15     ` Isaac Manjarres
2025-01-09 11:31       ` Lorenzo Stoakes
2025-01-09 18:14         ` Isaac Manjarres

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250107184804.4074147-2-isaacmanjarres@google.com \
    --to=isaacmanjarres@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=aliceryhl@google.com \
    --cc=jstultz@google.com \
    --cc=kaleshsingh@google.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=surenb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox