Merge patch series "file: simplify and harden cleanup handling"

Christian Brauner <brauner@kernel.org> says:

The cleanup guards rely on unnecessary infrastructure and make the code
more complicated to follow than it needs to be. Clean it up. Also shrink
the struct a bit and get rid of the unnecessary error variable. We can
just make this more ergonomic by using fdf->fd directly. Also expose the
struct itself as const and hide the actual underlying local variable
behind a UNIQUE_ID() named variable so we avoid risking shadow variables
and polluting callsites. Code generation wise the outcome is exactly the
same but simpler.

* patches from https://patch.msgid.link/20260917-work-file-fd_prepare-v1-0-b87534ca49f3@kernel.org:
  file: make struct fd_prepare const and kill its err field
  cleanup: remove CLASS_INIT()
  file: declare the FD_PREPARE() variable with __cleanup() directly
  file: simplify FD_PREPARE()

Link: https://patch.msgid.link/20260917-work-file-fd_prepare-v1-0-b87534ca49f3@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
This commit is contained in:
Christian Brauner 2026-09-23 09:28:02 +02:00
commit fae37d5e3c
No known key found for this signature in database
GPG Key ID: 91C61BC06578DCA2
20 changed files with 117 additions and 156 deletions

View File

@ -266,9 +266,9 @@ spufs_mkdir(struct inode *dir, struct dentry *dentry, unsigned int flags,
static int spufs_context_open(const struct path *path)
{
FD_PREPARE(fdf, 0, dentry_open(path, O_RDONLY, current_cred()));
if (fdf.err)
return fdf.err;
fd_prepare_file(fdf)->f_op = &spufs_context_fops;
if (fdf->fd < 0)
return fdf->fd;
fdf->file->f_op = &spufs_context_fops;
return fd_publish(fdf);
}
@ -499,9 +499,9 @@ static int spufs_gang_open(const struct path *path)
* in error path of *_open().
*/
FD_PREPARE(fdf, 0, dentry_open(path, O_RDONLY, current_cred()));
if (fdf.err)
return fdf.err;
fd_prepare_file(fdf)->f_op = &spufs_gang_fops;
if (fdf->fd < 0)
return fdf->fd;
fdf->file->f_op = &spufs_gang_fops;
return fd_publish(fdf);
}

View File

@ -377,11 +377,11 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
FD_PREPARE(fdf, O_RDONLY | O_CLOEXEC,
anon_inode_getfile("gpio-linehandle", &linehandle_fileops,
lh, O_RDONLY | O_CLOEXEC));
if (fdf.err)
return fdf.err;
if (fdf->fd < 0)
return fdf->fd;
retain_and_null_ptr(lh);
handlereq.fd = fd_prepare_fd(fdf);
handlereq.fd = fdf->fd;
if (copy_to_user(ip, &handlereq, sizeof(handlereq)))
return -EFAULT;
@ -1715,11 +1715,11 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
FD_PREPARE(fdf, O_RDONLY | O_CLOEXEC,
anon_inode_getfile("gpio-line", &line_fileops, lr,
O_RDONLY | O_CLOEXEC));
if (fdf.err)
return fdf.err;
if (fdf->fd < 0)
return fdf->fd;
retain_and_null_ptr(lr);
ulr.fd = fd_prepare_fd(fdf);
ulr.fd = fdf->fd;
if (copy_to_user(ip, &ulr, sizeof(ulr)))
return -EFAULT;
@ -2115,11 +2115,11 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
FD_PREPARE(fdf, O_RDONLY | O_CLOEXEC,
anon_inode_getfile("gpio-event", &lineevent_fileops, le,
O_RDONLY | O_CLOEXEC));
if (fdf.err)
return fdf.err;
if (fdf->fd < 0)
return fdf->fd;
retain_and_null_ptr(le);
eventreq.fd = fd_prepare_fd(fdf);
eventreq.fd = fdf->fd;
if (copy_to_user(ip, &eventreq, sizeof(eventreq)))
return -EFAULT;

View File

@ -543,8 +543,8 @@ msm_ioctl_perfcntr_config(struct drm_device *dev, void *data, struct drm_file *f
FD_PREPARE(fdf, O_CLOEXEC,
anon_inode_getfile("[msm_perfcntrs]", &stream_fops, stream, 0));
if (fdf.err)
return fdf.err;
if (fdf->fd < 0)
return fdf->fd;
INIT_WORK(&stream->sel_work, sel_worker);
kthread_init_work(&stream->sample_work, sample_worker);

View File

@ -316,15 +316,15 @@ int media_request_alloc(struct media_device *mdev, int *alloc_fd)
FD_PREPARE(fdf, O_CLOEXEC,
anon_inode_getfile("request", &request_fops, NULL,
O_CLOEXEC));
if (fdf.err) {
ret = fdf.err;
if (fdf->fd < 0) {
ret = fdf->fd;
goto err_free_req;
}
fd_prepare_file(fdf)->private_data = req;
fdf->file->private_data = req;
snprintf(req->debug_str, sizeof(req->debug_str), "%u:%d",
atomic_inc_return(&mdev->request_id), fd_prepare_fd(fdf));
atomic_inc_return(&mdev->request_id), fdf->fd);
atomic_inc(&mdev->num_requests);
dev_dbg(mdev->dev, "request: allocated %s\n", req->debug_str);

View File

@ -724,9 +724,9 @@ static int ntsync_obj_get_fd(struct ntsync_obj *obj)
{
FD_PREPARE(fdf, O_CLOEXEC,
anon_inode_getfile("ntsync", &ntsync_obj_fops, obj, O_RDWR));
if (fdf.err)
return fdf.err;
obj->file = fd_prepare_file(fdf);
if (fdf->fd < 0)
return fdf->fd;
obj->file = fdf->file;
return fd_publish(fdf);
}

View File

@ -403,8 +403,8 @@ static int do_eventfd(unsigned int count, int flags)
FD_PREPARE(fdf, flags,
anon_inode_getfile_fmode("[eventfd]", &eventfd_fops, ctx,
flags, FMODE_NOWAIT));
if (fdf.err)
return fdf.err;
if (fdf->fd < 0)
return fdf->fd;
ctx->id = ida_alloc(&eventfd_ida, GFP_KERNEL);
retain_and_null_ptr(ctx);

View File

@ -2514,11 +2514,11 @@ static int do_epoll_create(int flags)
FD_PREPARE(fdf, O_RDWR | (flags & O_CLOEXEC),
anon_inode_getfile("[eventpoll]", &eventpoll_fops, ep,
O_RDWR | (flags & O_CLOEXEC)));
if (fdf.err) {
if (fdf->fd < 0) {
ep_clear_and_put(ep);
return fdf.err;
return fdf->fd;
}
ep->file = fd_prepare_file(fdf);
ep->file = fdf->file;
return fd_publish(fdf);
}

View File

@ -1391,17 +1391,17 @@ int receive_fd(struct file *file, int __user *ufd, unsigned int o_flags)
return error;
FD_PREPARE(fdf, o_flags, file);
if (fdf.err)
return fdf.err;
if (fdf->fd < 0)
return fdf->fd;
get_file(file);
if (ufd) {
error = put_user(fd_prepare_fd(fdf), ufd);
error = put_user(fdf->fd, ufd);
if (error)
return error;
}
__receive_sock(fd_prepare_file(fdf));
__receive_sock(fdf->file);
return fd_publish(fdf);
}
EXPORT_SYMBOL_GPL(receive_fd);

View File

@ -4544,16 +4544,16 @@ SYSCALL_DEFINE3(fsmount, int, fs_fd, unsigned int, flags,
FD_PREPARE(fdf, (flags & FSMOUNT_CLOEXEC) ? O_CLOEXEC : 0,
dentry_open(&new_path, O_PATH, fc->cred));
if (fdf.err) {
if (fdf->fd < 0) {
dissolve_on_fput(new_path.mnt);
return fdf.err;
return fdf->fd;
}
/*
* Attach to an apparent O_PATH fd with a note that we
* need to unmount it, not just simply put it.
*/
fd_prepare_file(fdf)->f_mode |= FMODE_NEED_UNMOUNT;
fdf->file->f_mode |= FMODE_NEED_UNMOUNT;
return fd_publish(fdf);
}
@ -5198,12 +5198,12 @@ SYSCALL_DEFINE5(open_tree_attr, int, dfd, const char __user *, filename,
return -EINVAL;
FD_PREPARE(fdf, flags, vfs_open_tree(dfd, filename, flags));
if (fdf.err)
return fdf.err;
if (fdf->fd < 0)
return fdf->fd;
if (uattr) {
struct mount_kattr kattr = {};
struct file *file = fd_prepare_file(fdf);
struct file *file = fdf->file;
int ret;
if (flags & OPEN_TREE_CLONE)

View File

@ -348,8 +348,8 @@ static long ns_ioctl(struct file *filp, unsigned int ioctl,
return ret;
FD_PREPARE(fdf, O_CLOEXEC, dentry_open(&path, O_RDONLY, current_cred()));
if (fdf.err)
return fdf.err;
if (fdf->fd < 0)
return fdf->fd;
/*
* If @uinfo is passed return all information about the
* mount namespace as well.

View File

@ -272,11 +272,11 @@ xfs_open_by_handle(
path.mnt = mntget(parfilp->f_path.mnt);
FD_PREPARE(fdf, 0, dentry_open(&path, hreq->oflags, cred));
if (fdf.err)
return fdf.err;
if (fdf->fd < 0)
return fdf->fd;
if (S_ISREG(inode->i_mode)) {
struct file *filp = fd_prepare_file(fdf);
struct file *filp = fdf->file;
filp->f_flags |= O_NOATIME;
filp->f_mode |= FMODE_NOCMTIME;

View File

@ -261,10 +261,6 @@ const volatile void * __must_check_fn(const volatile void *val)
* CLASS(name, var)(args...):
* declare the variable @var as an instance of the named class
*
* CLASS_INIT(name, var, init_expr):
* declare the variable @var as an instance of the named class with
* custom initialization expression.
*
* Ex.
*
* DEFINE_CLASS(fdget, struct fd, fdput(_T), fdget(fd), int fd)
@ -302,9 +298,6 @@ static __always_inline class_##_name##_t class_##_name##ext##_constructor(_init_
class_##_name##_t var __cleanup(class_##_name##_destructor) = \
class_##_name##_constructor
#define CLASS_INIT(_name, _var, _init_expr) \
class_##_name##_t _var __cleanup(class_##_name##_destructor) = (_init_expr)
#define __scoped_class(_name, var, _label, args...) \
for (CLASS(_name, var)(args); ; ({ goto _label; })) \
if (0) { \

View File

@ -12,6 +12,7 @@
#include <linux/errno.h>
#include <linux/cleanup.h>
#include <linux/err.h>
#include <linux/vfsdebug.h>
struct file;
@ -129,117 +130,84 @@ extern unsigned int sysctl_nr_open_min, sysctl_nr_open_max;
/*
* fd_prepare: Combined fd + file allocation cleanup class.
* @err: Error code to indicate if allocation succeeded.
* @__fd: Allocated fd (may not be accessed directly)
* @__file: Allocated struct file pointer (may not be accessed directly)
* @fd: Allocated fd
* @file: Allocated struct file pointer
*
* Allocates an fd and a file together. On error paths, automatically cleans
* up whichever resource was successfully allocated. Allows flexible file
* allocation with different functions per usage.
*
* Do not use directly.
* Do not declare directly, use FD_PREPARE().
*/
struct fd_prepare {
s32 err;
s32 __fd; /* do not access directly */
struct file *__file; /* do not access directly */
int fd;
struct file *file;
};
/* Typedef for fd_prepare cleanup guards. */
typedef struct fd_prepare class_fd_prepare_t;
/*
* Accessors for fd_prepare class members.
* _Generic() is used for zero-cost type safety.
*/
#define fd_prepare_fd(_fdf) \
(_Generic((_fdf), struct fd_prepare: (_fdf).__fd))
#define fd_prepare_file(_fdf) \
(_Generic((_fdf), struct fd_prepare: (_fdf).__file))
/* Do not use directly. */
static inline void class_fd_prepare_destructor(const struct fd_prepare *fdf)
static __always_inline void __fd_prepare_cleanup(const struct fd_prepare *fdf)
{
if (unlikely(fdf->__fd >= 0))
put_unused_fd(fdf->__fd);
if (unlikely(!IS_ERR_OR_NULL(fdf->__file)))
fput(fdf->__file);
if (unlikely(fdf->fd >= 0)) {
put_unused_fd(fdf->fd);
fput(fdf->file);
}
}
/* Do not use directly. */
static inline int class_fd_prepare_lock_err(const struct fd_prepare *fdf)
static __always_inline struct fd_prepare __fd_prepare(int fd, struct file *file)
{
if (unlikely(fdf->err))
return fdf->err;
if (unlikely(fdf->__fd < 0))
return fdf->__fd;
if (unlikely(IS_ERR(fdf->__file)))
return PTR_ERR(fdf->__file);
if (unlikely(!fdf->__file))
return -ENOMEM;
return 0;
if (fd >= 0 && IS_ERR_OR_NULL(file)) {
int err = file ? PTR_ERR(file) : -ENOMEM;
put_unused_fd(fd);
fd = err;
file = NULL;
}
return (struct fd_prepare){ .fd = fd, .file = file };
}
/*
* __FD_PREPARE_INIT - Helper to initialize fd_prepare class.
* @_fd_flags: flags for get_unused_fd_flags()
* @_file_owned: expression that returns struct file *
* FD_PREPARE - Declare and initialize an fd_prepare instance.
*
* Returns a struct fd_prepare with fd, file, and err set.
* If fd allocation fails, fd will be negative and err will be set. If
* fd succeeds but file_init_expr fails, file will be ERR_PTR and err
* will be set. The err field is the single source of truth for error
* checking.
*/
#define __FD_PREPARE_INIT(_fd_flags, _file_owned) \
({ \
struct fd_prepare fdf = { \
.__fd = get_unused_fd_flags((_fd_flags)), \
}; \
if (likely(fdf.__fd >= 0)) \
fdf.__file = (_file_owned); \
fdf.err = ACQUIRE_ERR(fd_prepare, &fdf); \
fdf; \
})
/*
* FD_PREPARE - Macro to declare and initialize an fd_prepare variable.
* This allocates a new fd and only evaluates @_file_owned if the
* allocation succeeded. Cleanup happens when the variable goes out of
* scope and the guard releases whichever of the descriptor and the file
* was allocated. If fd_publish() was called the fd and file are
* published and cleanup becomes a nop.
*
* Declares and initializes an fd_prepare variable with automatic
* cleanup. No separate scope required - cleanup happens when variable
* goes out of scope.
*
* @_fdf: name of struct fd_prepare variable to define
* @_fdf: name of the const struct fd_prepare pointer to define
* @_fd_flags: flags for get_unused_fd_flags()
* @_file_owned: struct file to take ownership of (can be expression)
*/
#define __FD_PREPARE(_guard, _fdf, _fd_flags, _file_owned) \
struct fd_prepare _guard __cleanup(__fd_prepare_cleanup) = ({ \
int __fd = get_unused_fd_flags(_fd_flags); \
__fd_prepare(__fd, __fd < 0 ? NULL : (_file_owned)); \
}); \
const struct fd_prepare *const _fdf = &_guard
#define FD_PREPARE(_fdf, _fd_flags, _file_owned) \
CLASS_INIT(fd_prepare, _fdf, __FD_PREPARE_INIT(_fd_flags, _file_owned))
__FD_PREPARE(__UNIQUE_ID(fd_prepare), _fdf, _fd_flags, _file_owned)
/*
* fd_publish - Publish prepared fd and file to the fd table.
* @_fdf: struct fd_prepare variable
* @fdf: struct fd_prepare pointer defined by FD_PREPARE()
*/
#define fd_publish(_fdf) \
({ \
struct fd_prepare *fdp = &(_fdf); \
VFS_WARN_ON_ONCE(fdp->err); \
VFS_WARN_ON_ONCE(fdp->__fd < 0); \
VFS_WARN_ON_ONCE(IS_ERR_OR_NULL(fdp->__file)); \
fd_install(fdp->__fd, fdp->__file); \
retain_and_null_ptr(fdp->__file); \
take_fd(fdp->__fd); \
})
static __always_inline int fd_publish(const struct fd_prepare *fdf)
{
/* Callers only get a const view, the guard itself is writable. */
struct fd_prepare *guard = (struct fd_prepare *)fdf;
VFS_WARN_ON_ONCE(guard->fd < 0);
fd_install(guard->fd, guard->file);
return take_fd(guard->fd);
}
/* Do not use directly. */
#define __FD_ADD(_fdf, _fd_flags, _file_owned) \
({ \
FD_PREPARE(_fdf, _fd_flags, _file_owned); \
s32 ret = _fdf.err; \
if (likely(!ret)) \
ret = fd_publish(_fdf); \
ret; \
#define __FD_ADD(_fdf, _fd_flags, _file_owned) \
({ \
FD_PREPARE(_fdf, _fd_flags, _file_owned); \
_fdf->fd < 0 ? _fdf->fd : fd_publish(_fdf); \
})
/*

View File

@ -257,17 +257,17 @@ static int io_create_mock_file(struct io_uring_cmd *cmd, unsigned int issue_flag
FD_PREPARE(fdf, O_RDWR | O_CLOEXEC,
anon_inode_create_getfile("[io_uring_mock]", fops, mf,
O_RDWR | O_CLOEXEC, NULL));
if (fdf.err)
return fdf.err;
if (fdf->fd < 0)
return fdf->fd;
retain_and_null_ptr(mf);
file = fd_prepare_file(fdf);
file = fdf->file;
file->f_mode |= FMODE_READ | FMODE_CAN_READ | FMODE_WRITE |
FMODE_CAN_WRITE | FMODE_LSEEK;
if (mc.flags & IORING_MOCK_CREATE_F_SUPPORT_NOWAIT)
file->f_mode |= FMODE_NOWAIT;
mc.out_fd = fd_prepare_fd(fdf);
mc.out_fd = fdf->fd;
if (copy_to_user(uarg, &mc, uarg_size))
return -EFAULT;

View File

@ -643,11 +643,11 @@ int bpf_iter_new_fd(struct bpf_link *link)
flags = O_RDONLY | O_CLOEXEC;
FD_PREPARE(fdf, flags, anon_inode_getfile("bpf_iter", &bpf_iter_fops, NULL, flags));
if (fdf.err)
return fdf.err;
if (fdf->fd < 0)
return fdf->fd;
iter_link = container_of(link, struct bpf_iter_link, link);
err = prepare_seq_file(fd_prepare_file(fdf), iter_link);
err = prepare_seq_file(fdf->file, iter_link);
if (err)
return err; /* Automatic cleanup handles fput */

View File

@ -169,8 +169,8 @@ int bpf_token_create(union bpf_attr *attr)
FD_PREPARE(fdf, O_CLOEXEC,
alloc_file_pseudo(inode, path.mnt, BPF_TOKEN_INODE_NAME,
O_RDWR, &bpf_token_fops));
if (fdf.err)
return fdf.err;
if (fdf->fd < 0)
return fdf->fd;
token = kzalloc_obj(*token, GFP_USER);
if (!token)
@ -190,7 +190,7 @@ int bpf_token_create(union bpf_attr *attr)
return err;
get_user_ns(token->userns);
fd_prepare_file(fdf)->private_data = no_free_ptr(token);
fdf->file->private_data = no_free_ptr(token);
return fd_publish(fdf);
}

View File

@ -4809,12 +4809,12 @@ static int new_userfaultfd(int flags)
anon_inode_create_getfile("[userfaultfd]", &userfaultfd_fops, ctx,
O_RDONLY | (flags & UFFD_SHARED_FCNTL_FLAGS),
NULL));
if (fdf.err)
return fdf.err;
if (fdf->fd < 0)
return fdf->fd;
/* prevent the mm struct to be freed */
mmgrab(ctx->mm);
fd_prepare_file(fdf)->f_mode |= FMODE_NOWAIT;
fdf->file->f_mode |= FMODE_NOWAIT;
retain_and_null_ptr(ctx);
return fd_publish(fdf);
}

View File

@ -364,14 +364,14 @@ int scm_recv_one_fd(struct file *f, int __user *ufd, unsigned int flags,
return notrunc ? put_user(error, ufd) : error;
FD_PREPARE(fdf, flags, get_file(f));
if (fdf.err)
return fdf.err;
if (fdf->fd < 0)
return fdf->fd;
error = put_user(fd_prepare_fd(fdf), ufd);
error = put_user(fdf->fd, ufd);
if (error)
return error;
__receive_sock(fd_prepare_file(fdf));
__receive_sock(fdf->file);
return fd_publish(fdf);
}

View File

@ -107,17 +107,17 @@ int handshake_nl_accept_doit(struct sk_buff *skb, struct genl_info *info)
req = handshake_req_next(hn, class);
if (req) {
FD_PREPARE(fdf, O_CLOEXEC, req->hr_file);
if (fdf.err) {
if (fdf->fd < 0) {
fput(req->hr_file); /* drop ref from handshake_req_next() */
err = fdf.err;
err = fdf->fd;
goto out_complete;
}
err = req->hr_proto->hp_accept(req, info, fd_prepare_fd(fdf));
err = req->hr_proto->hp_accept(req, info, fdf->fd);
if (err)
goto out_complete; /* Automatic cleanup handles fput */
trace_handshake_cmd_accept(net, req, req->hr_sk, fd_prepare_fd(fdf));
trace_handshake_cmd_accept(net, req, req->hr_sk, fdf->fd);
fd_publish(fdf);
return 0;
}

View File

@ -1580,10 +1580,10 @@ static int kcm_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
struct kcm_clone info;
FD_PREPARE(fdf, 0, kcm_clone(sock));
if (fdf.err)
return fdf.err;
if (fdf->fd < 0)
return fdf->fd;
info.fd = fd_prepare_fd(fdf);
info.fd = fdf->fd;
if (copy_to_user((void __user *)arg, &info, sizeof(info)))
return -EFAULT;