diff mbox series

[05/39] regularize emptiness checks in fini_module(2) and vfs_dedupe_file_range()

Message ID 20240730051625.14349-5-viro@kernel.org (mailing list archive)
State New
Headers show
Series [01/39] memcg_write_event_control(): fix a user-triggerable oops | expand

Commit Message

Al Viro July 30, 2024, 5:15 a.m. UTC 
From: Al Viro <viro@zeniv.linux.org.uk>

Currently all emptiness checks are done as fd_file(...) in boolean
context (usually something like if (!fd_file(f))...); those will be
taken care of later.

However, there's a couple of places where we do those checks as
'store fd_file(...) into a variable, then check if this variable is
NULL' and those are harder to spot.

Get rid of those now.

use fd_empty() instead of extracting file and then checking it for NULL.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/remap_range.c     | 5 ++---
 kernel/module/main.c | 4 +++-
 2 files changed, 5 insertions(+), 4 deletions(-)

Comments

Christian Brauner Aug. 7, 2024, 10:10 a.m. UTC | #1 
On Tue, Jul 30, 2024 at 01:15:51AM GMT, viro@kernel.org wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> Currently all emptiness checks are done as fd_file(...) in boolean
> context (usually something like if (!fd_file(f))...); those will be
> taken care of later.
> 
> However, there's a couple of places where we do those checks as
> 'store fd_file(...) into a variable, then check if this variable is
> NULL' and those are harder to spot.
> 
> Get rid of those now.
> 
> use fd_empty() instead of extracting file and then checking it for NULL.
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---

Reviewed-by: Christian Brauner <brauner@kernel.org>
diff mbox series

Patch

diff --git a/fs/remap_range.c b/fs/remap_range.c
index 4403d5c68fcb..017d0d1ea6c9 100644
--- a/fs/remap_range.c
+++ b/fs/remap_range.c
@@ -537,9 +537,8 @@  int vfs_dedupe_file_range(struct file *file, struct file_dedupe_range *same)
 
 	for (i = 0, info = same->info; i < count; i++, info++) {
 		struct fd dst_fd = fdget(info->dest_fd);
-		struct file *dst_file = fd_file(dst_fd);
 
-		if (!dst_file) {
+		if (fd_empty(dst_fd)) {
 			info->status = -EBADF;
 			goto next_loop;
 		}
@@ -549,7 +548,7 @@  int vfs_dedupe_file_range(struct file *file, struct file_dedupe_range *same)
 			goto next_fdput;
 		}
 
-		deduped = vfs_dedupe_file_range_one(file, off, dst_file,
+		deduped = vfs_dedupe_file_range_one(file, off, fd_file(dst_fd),
 						    info->dest_offset, len,
 						    REMAP_FILE_CAN_SHORTEN);
 		if (deduped == -EBADE)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 6ed334eecc14..93cc9fea9d9d 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3180,7 +3180,7 @@  static int idempotent_init_module(struct file *f, const char __user * uargs, int
 {
 	struct idempotent idem;
 
-	if (!f || !(f->f_mode & FMODE_READ))
+	if (!(f->f_mode & FMODE_READ))
 		return -EBADF;
 
 	/* See if somebody else is doing the operation? */
@@ -3211,6 +3211,8 @@  SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
 		return -EINVAL;
 
 	f = fdget(fd);
+	if (fd_empty(f))
+		return -EBADF;
 	err = idempotent_init_module(fd_file(f), uargs, flags);
 	fdput(f);
 	return err;