* [PATCH v5 2/2] fs: Replace user_access_{begin/end} by scoped user access
2026-03-24 11:41 [PATCH v5 1/2] readdir: Introduce dirent_size() Christophe Leroy (CS GROUP)
@ 2026-03-24 11:41 ` Christophe Leroy (CS GROUP)
2026-03-24 12:08 ` [PATCH v5 1/2] readdir: Introduce dirent_size() David Laight
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Christophe Leroy (CS GROUP) @ 2026-03-24 11:41 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Linus Torvalds,
Thomas Gleixner, David Laight
Cc: Christophe Leroy (CS GROUP), linux-fsdevel, linux-kernel
Scoped user access reduces code complexity and seamlessly bring
masked user access on architectures that support it.
Replace user_access_begin/user_access_end blocks by
scoped user access.
Signed-off-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
---
v2:
- Fix build failure with CONFIG_COMPAT
- Handled checkpatch.pl output
v3:
- Fix again build failure with CONFIG_COMPAT. I was obviously too tired when I sent out v2.
v4:
- Introduced local size var based on Linus comment on readability. Not a dirent_size() helper as suggested as both sites are different.
v5:
- Just rely on the macro added in previous patch.
---
fs/readdir.c | 88 +++++++++++++++++++++-------------------------------
fs/select.c | 35 +++++++++------------
2 files changed, 49 insertions(+), 74 deletions(-)
diff --git a/fs/readdir.c b/fs/readdir.c
index fb910dc2f52b..76bb1ae3a450 100644
--- a/fs/readdir.c
+++ b/fs/readdir.c
@@ -200,16 +200,13 @@ static bool fillonedir(struct dir_context *ctx, const char *name, int namlen,
}
buf->result++;
dirent = buf->dirent;
- if (!user_write_access_begin(dirent, dirent_size(dirent, namlen + 1)))
- goto efault;
- unsafe_put_user(d_ino, &dirent->d_ino, efault_end);
- unsafe_put_user(offset, &dirent->d_offset, efault_end);
- unsafe_put_user(namlen, &dirent->d_namlen, efault_end);
- unsafe_copy_dirent_name(dirent->d_name, name, namlen, efault_end);
- user_write_access_end();
+ scoped_user_write_access_size(dirent, dirent_size(dirent, namlen + 1), efault) {
+ unsafe_put_user(d_ino, &dirent->d_ino, efault);
+ unsafe_put_user(offset, &dirent->d_offset, efault);
+ unsafe_put_user(namlen, &dirent->d_namlen, efault);
+ unsafe_copy_dirent_name(dirent->d_name, name, namlen, efault);
+ }
return true;
-efault_end:
- user_write_access_end();
efault:
buf->result = -EFAULT;
return false;
@@ -286,23 +283,19 @@ static bool filldir(struct dir_context *ctx, const char *name, int namlen,
return false;
dirent = buf->current_dir;
prev = (void __user *) dirent - prev_reclen;
- if (!user_write_access_begin(prev, reclen + prev_reclen))
- goto efault;
-
- /* This might be 'dirent->d_off', but if so it will get overwritten */
- unsafe_put_user(offset, &prev->d_off, efault_end);
- unsafe_put_user(d_ino, &dirent->d_ino, efault_end);
- unsafe_put_user(reclen, &dirent->d_reclen, efault_end);
- unsafe_put_user(d_type, (char __user *) dirent + reclen - 1, efault_end);
- unsafe_copy_dirent_name(dirent->d_name, name, namlen, efault_end);
- user_write_access_end();
+ scoped_user_write_access_size(prev, reclen + prev_reclen, efault) {
+ /* This might be 'dirent->d_off', but if so it will get overwritten */
+ unsafe_put_user(offset, &prev->d_off, efault);
+ unsafe_put_user(d_ino, &dirent->d_ino, efault);
+ unsafe_put_user(reclen, &dirent->d_reclen, efault);
+ unsafe_put_user(d_type, (char __user *)dirent + reclen - 1, efault);
+ unsafe_copy_dirent_name(dirent->d_name, name, namlen, efault);
+ }
buf->current_dir = (void __user *)dirent + reclen;
buf->prev_reclen = reclen;
ctx->count -= reclen;
return true;
-efault_end:
- user_write_access_end();
efault:
buf->error = -EFAULT;
return false;
@@ -369,24 +362,20 @@ static bool filldir64(struct dir_context *ctx, const char *name, int namlen,
return false;
dirent = buf->current_dir;
prev = (void __user *)dirent - prev_reclen;
- if (!user_write_access_begin(prev, reclen + prev_reclen))
- goto efault;
-
- /* This might be 'dirent->d_off', but if so it will get overwritten */
- unsafe_put_user(offset, &prev->d_off, efault_end);
- unsafe_put_user(ino, &dirent->d_ino, efault_end);
- unsafe_put_user(reclen, &dirent->d_reclen, efault_end);
- unsafe_put_user(d_type, &dirent->d_type, efault_end);
- unsafe_copy_dirent_name(dirent->d_name, name, namlen, efault_end);
- user_write_access_end();
+ scoped_user_write_access_size(prev, reclen + prev_reclen, efault) {
+ /* This might be 'dirent->d_off', but if so it will get overwritten */
+ unsafe_put_user(offset, &prev->d_off, efault);
+ unsafe_put_user(ino, &dirent->d_ino, efault);
+ unsafe_put_user(reclen, &dirent->d_reclen, efault);
+ unsafe_put_user(d_type, &dirent->d_type, efault);
+ unsafe_copy_dirent_name(dirent->d_name, name, namlen, efault);
+ }
buf->prev_reclen = reclen;
buf->current_dir = (void __user *)dirent + reclen;
ctx->count -= reclen;
return true;
-efault_end:
- user_write_access_end();
efault:
buf->error = -EFAULT;
return false;
@@ -458,16 +447,13 @@ static bool compat_fillonedir(struct dir_context *ctx, const char *name,
}
buf->result++;
dirent = buf->dirent;
- if (!user_write_access_begin(dirent, dirent_size(dirent, namlen + 1)))
- goto efault;
- unsafe_put_user(d_ino, &dirent->d_ino, efault_end);
- unsafe_put_user(offset, &dirent->d_offset, efault_end);
- unsafe_put_user(namlen, &dirent->d_namlen, efault_end);
- unsafe_copy_dirent_name(dirent->d_name, name, namlen, efault_end);
- user_write_access_end();
+ scoped_user_write_access_size(dirent, dirent_size(dirent, namlen + 1), efault) {
+ unsafe_put_user(d_ino, &dirent->d_ino, efault);
+ unsafe_put_user(offset, &dirent->d_offset, efault);
+ unsafe_put_user(namlen, &dirent->d_namlen, efault);
+ unsafe_copy_dirent_name(dirent->d_name, name, namlen, efault);
+ }
return true;
-efault_end:
- user_write_access_end();
efault:
buf->result = -EFAULT;
return false;
@@ -538,22 +524,18 @@ static bool compat_filldir(struct dir_context *ctx, const char *name, int namlen
return false;
dirent = buf->current_dir;
prev = (void __user *) dirent - prev_reclen;
- if (!user_write_access_begin(prev, reclen + prev_reclen))
- goto efault;
-
- unsafe_put_user(offset, &prev->d_off, efault_end);
- unsafe_put_user(d_ino, &dirent->d_ino, efault_end);
- unsafe_put_user(reclen, &dirent->d_reclen, efault_end);
- unsafe_put_user(d_type, (char __user *) dirent + reclen - 1, efault_end);
- unsafe_copy_dirent_name(dirent->d_name, name, namlen, efault_end);
- user_write_access_end();
+ scoped_user_write_access_size(prev, reclen + prev_reclen, efault) {
+ unsafe_put_user(offset, &prev->d_off, efault);
+ unsafe_put_user(d_ino, &dirent->d_ino, efault);
+ unsafe_put_user(reclen, &dirent->d_reclen, efault);
+ unsafe_put_user(d_type, (char __user *)dirent + reclen - 1, efault);
+ unsafe_copy_dirent_name(dirent->d_name, name, namlen, efault);
+ }
buf->prev_reclen = reclen;
buf->current_dir = (void __user *)dirent + reclen;
ctx->count -= reclen;
return true;
-efault_end:
- user_write_access_end();
efault:
buf->error = -EFAULT;
return false;
diff --git a/fs/select.c b/fs/select.c
index e0244dbe4429..75978b18f48f 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -1004,17 +1004,17 @@ static int do_sys_poll(struct pollfd __user *ufds, unsigned int nfds,
fdcount = do_poll(head, &table, end_time);
poll_freewait(&table);
- if (!user_write_access_begin(ufds, nfds * sizeof(*ufds)))
- goto out_fds;
+ scoped_user_write_access_size(ufds, nfds * sizeof(*ufds), out_fds) {
+ struct pollfd __user *_ufds = ufds;
- for (walk = head; walk; walk = walk->next) {
- struct pollfd *fds = walk->entries;
- unsigned int j;
+ for (walk = head; walk; walk = walk->next) {
+ struct pollfd *fds = walk->entries;
+ unsigned int j;
- for (j = walk->len; j; fds++, ufds++, j--)
- unsafe_put_user(fds->revents, &ufds->revents, Efault);
- }
- user_write_access_end();
+ for (j = walk->len; j; fds++, _ufds++, j--)
+ unsafe_put_user(fds->revents, &_ufds->revents, out_fds);
+ }
+ }
err = fdcount;
out_fds:
@@ -1026,11 +1026,6 @@ static int do_sys_poll(struct pollfd __user *ufds, unsigned int nfds,
}
return err;
-
-Efault:
- user_write_access_end();
- err = -EFAULT;
- goto out_fds;
}
static long do_restart_poll(struct restart_block *restart_block)
@@ -1338,15 +1333,13 @@ static inline int get_compat_sigset_argpack(struct compat_sigset_argpack *to,
struct compat_sigset_argpack __user *from)
{
if (from) {
- if (!user_read_access_begin(from, sizeof(*from)))
- return -EFAULT;
- unsafe_get_user(to->p, &from->p, Efault);
- unsafe_get_user(to->size, &from->size, Efault);
- user_read_access_end();
+ scoped_user_read_access(from, efault) {
+ unsafe_get_user(to->p, &from->p, efault);
+ unsafe_get_user(to->size, &from->size, efault);
+ }
}
return 0;
-Efault:
- user_read_access_end();
+efault:
return -EFAULT;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v5 1/2] readdir: Introduce dirent_size()
2026-03-24 11:41 [PATCH v5 1/2] readdir: Introduce dirent_size() Christophe Leroy (CS GROUP)
2026-03-24 11:41 ` [PATCH v5 2/2] fs: Replace user_access_{begin/end} by scoped user access Christophe Leroy (CS GROUP)
@ 2026-03-24 12:08 ` David Laight
2026-03-24 13:46 ` Jan Kara
2026-03-24 15:07 ` Christian Brauner
3 siblings, 0 replies; 5+ messages in thread
From: David Laight @ 2026-03-24 12:08 UTC (permalink / raw)
To: Christophe Leroy (CS GROUP)
Cc: Alexander Viro, Christian Brauner, Jan Kara, Linus Torvalds,
Thomas Gleixner, linux-fsdevel, linux-kernel
On Tue, 24 Mar 2026 12:41:15 +0100
"Christophe Leroy (CS GROUP)" <chleroy@kernel.org> wrote:
> In several places in readdir.c there are calculations of the total size
> of a dirent, which contains a few fixed fields plus a name field with
> variable size. To add fun every dirent is of a slightly different type:
> - struct old_linux_dirent
> - struct linux_dirent
> - struct linux_dirent64
> - struct compat_old_linux_dirent
> - struct compat_linux_dirent
>
> Replace ugly size calculation by a macro called dirent_size() which
> calculates the size of the structure based on the pointed type and
> the name field len.
>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
Reviewed-by: David Laight <david.laight.linux@gmail.com>
> ---
> fs/readdir.c | 19 +++++++------------
> 1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/fs/readdir.c b/fs/readdir.c
> index 73707b6816e9..fb910dc2f52b 100644
> --- a/fs/readdir.c
> +++ b/fs/readdir.c
> @@ -22,6 +22,8 @@
> #include <linux/compat.h>
> #include <linux/uaccess.h>
>
> +#define dirent_size(dirent, len) offsetof(typeof(*(dirent)), d_name[len])
> +
> /*
> * Some filesystems were never converted to '->iterate_shared()'
> * and their directory iterators want the inode lock held for
> @@ -198,9 +200,7 @@ static bool fillonedir(struct dir_context *ctx, const char *name, int namlen,
> }
> buf->result++;
> dirent = buf->dirent;
> - if (!user_write_access_begin(dirent,
> - (unsigned long)(dirent->d_name + namlen + 1) -
> - (unsigned long)dirent))
> + if (!user_write_access_begin(dirent, dirent_size(dirent, namlen + 1)))
> goto efault;
> unsafe_put_user(d_ino, &dirent->d_ino, efault_end);
> unsafe_put_user(offset, &dirent->d_offset, efault_end);
> @@ -263,8 +263,7 @@ static bool filldir(struct dir_context *ctx, const char *name, int namlen,
> struct getdents_callback *buf =
> container_of(ctx, struct getdents_callback, ctx);
> unsigned long d_ino;
> - int reclen = ALIGN(offsetof(struct linux_dirent, d_name) + namlen + 2,
> - sizeof(long));
> + int reclen = ALIGN(dirent_size(dirent, namlen + 2), sizeof(long));
> int prev_reclen;
> unsigned int flags = d_type;
>
> @@ -352,8 +351,7 @@ static bool filldir64(struct dir_context *ctx, const char *name, int namlen,
> struct linux_dirent64 __user *dirent, *prev;
> struct getdents_callback64 *buf =
> container_of(ctx, struct getdents_callback64, ctx);
> - int reclen = ALIGN(offsetof(struct linux_dirent64, d_name) + namlen + 1,
> - sizeof(u64));
> + int reclen = ALIGN(dirent_size(dirent, namlen + 1), sizeof(u64));
> int prev_reclen;
> unsigned int flags = d_type;
>
> @@ -460,9 +458,7 @@ static bool compat_fillonedir(struct dir_context *ctx, const char *name,
> }
> buf->result++;
> dirent = buf->dirent;
> - if (!user_write_access_begin(dirent,
> - (unsigned long)(dirent->d_name + namlen + 1) -
> - (unsigned long)dirent))
> + if (!user_write_access_begin(dirent, dirent_size(dirent, namlen + 1)))
> goto efault;
> unsafe_put_user(d_ino, &dirent->d_ino, efault_end);
> unsafe_put_user(offset, &dirent->d_offset, efault_end);
> @@ -519,8 +515,7 @@ static bool compat_filldir(struct dir_context *ctx, const char *name, int namlen
> struct compat_getdents_callback *buf =
> container_of(ctx, struct compat_getdents_callback, ctx);
> compat_ulong_t d_ino;
> - int reclen = ALIGN(offsetof(struct compat_linux_dirent, d_name) +
> - namlen + 2, sizeof(compat_long_t));
> + int reclen = ALIGN(dirent_size(dirent, namlen + 2), sizeof(compat_long_t));
> int prev_reclen;
> unsigned int flags = d_type;
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v5 1/2] readdir: Introduce dirent_size()
2026-03-24 11:41 [PATCH v5 1/2] readdir: Introduce dirent_size() Christophe Leroy (CS GROUP)
2026-03-24 11:41 ` [PATCH v5 2/2] fs: Replace user_access_{begin/end} by scoped user access Christophe Leroy (CS GROUP)
2026-03-24 12:08 ` [PATCH v5 1/2] readdir: Introduce dirent_size() David Laight
@ 2026-03-24 13:46 ` Jan Kara
2026-03-24 15:07 ` Christian Brauner
3 siblings, 0 replies; 5+ messages in thread
From: Jan Kara @ 2026-03-24 13:46 UTC (permalink / raw)
To: Christophe Leroy (CS GROUP)
Cc: Alexander Viro, Christian Brauner, Jan Kara, Linus Torvalds,
Thomas Gleixner, David Laight, linux-fsdevel, linux-kernel
On Tue 24-03-26 12:41:15, Christophe Leroy (CS GROUP) wrote:
> In several places in readdir.c there are calculations of the total size
> of a dirent, which contains a few fixed fields plus a name field with
> variable size. To add fun every dirent is of a slightly different type:
> - struct old_linux_dirent
> - struct linux_dirent
> - struct linux_dirent64
> - struct compat_old_linux_dirent
> - struct compat_linux_dirent
>
> Replace ugly size calculation by a macro called dirent_size() which
> calculates the size of the structure based on the pointed type and
> the name field len.
>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/readdir.c | 19 +++++++------------
> 1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/fs/readdir.c b/fs/readdir.c
> index 73707b6816e9..fb910dc2f52b 100644
> --- a/fs/readdir.c
> +++ b/fs/readdir.c
> @@ -22,6 +22,8 @@
> #include <linux/compat.h>
> #include <linux/uaccess.h>
>
> +#define dirent_size(dirent, len) offsetof(typeof(*(dirent)), d_name[len])
> +
> /*
> * Some filesystems were never converted to '->iterate_shared()'
> * and their directory iterators want the inode lock held for
> @@ -198,9 +200,7 @@ static bool fillonedir(struct dir_context *ctx, const char *name, int namlen,
> }
> buf->result++;
> dirent = buf->dirent;
> - if (!user_write_access_begin(dirent,
> - (unsigned long)(dirent->d_name + namlen + 1) -
> - (unsigned long)dirent))
> + if (!user_write_access_begin(dirent, dirent_size(dirent, namlen + 1)))
> goto efault;
> unsafe_put_user(d_ino, &dirent->d_ino, efault_end);
> unsafe_put_user(offset, &dirent->d_offset, efault_end);
> @@ -263,8 +263,7 @@ static bool filldir(struct dir_context *ctx, const char *name, int namlen,
> struct getdents_callback *buf =
> container_of(ctx, struct getdents_callback, ctx);
> unsigned long d_ino;
> - int reclen = ALIGN(offsetof(struct linux_dirent, d_name) + namlen + 2,
> - sizeof(long));
> + int reclen = ALIGN(dirent_size(dirent, namlen + 2), sizeof(long));
> int prev_reclen;
> unsigned int flags = d_type;
>
> @@ -352,8 +351,7 @@ static bool filldir64(struct dir_context *ctx, const char *name, int namlen,
> struct linux_dirent64 __user *dirent, *prev;
> struct getdents_callback64 *buf =
> container_of(ctx, struct getdents_callback64, ctx);
> - int reclen = ALIGN(offsetof(struct linux_dirent64, d_name) + namlen + 1,
> - sizeof(u64));
> + int reclen = ALIGN(dirent_size(dirent, namlen + 1), sizeof(u64));
> int prev_reclen;
> unsigned int flags = d_type;
>
> @@ -460,9 +458,7 @@ static bool compat_fillonedir(struct dir_context *ctx, const char *name,
> }
> buf->result++;
> dirent = buf->dirent;
> - if (!user_write_access_begin(dirent,
> - (unsigned long)(dirent->d_name + namlen + 1) -
> - (unsigned long)dirent))
> + if (!user_write_access_begin(dirent, dirent_size(dirent, namlen + 1)))
> goto efault;
> unsafe_put_user(d_ino, &dirent->d_ino, efault_end);
> unsafe_put_user(offset, &dirent->d_offset, efault_end);
> @@ -519,8 +515,7 @@ static bool compat_filldir(struct dir_context *ctx, const char *name, int namlen
> struct compat_getdents_callback *buf =
> container_of(ctx, struct compat_getdents_callback, ctx);
> compat_ulong_t d_ino;
> - int reclen = ALIGN(offsetof(struct compat_linux_dirent, d_name) +
> - namlen + 2, sizeof(compat_long_t));
> + int reclen = ALIGN(dirent_size(dirent, namlen + 2), sizeof(compat_long_t));
> int prev_reclen;
> unsigned int flags = d_type;
>
> --
> 2.49.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 5+ messages in thread