| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Y710pFYI8rBX9Eqe@bfoster>
Date: Tue, 10 Jan 2023 09:22:28 -0500
From: Brian Foster <bfoster@...hat.com>
To: Nhat Pham <nphamcs@...il.com>
Cc: akpm@...ux-foundation.org, hannes@...xchg.org, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, willy@...radead.org,
kernel-team@...a.com
Subject: Re: [PATCH v5 2/3] cachestat: implement cachestat syscall
On Wed, Jan 04, 2023 at 03:11:26PM -0800, Nhat Pham wrote:
> Implement a new syscall that queries cache state of a file and
> summarizes the number of cached pages, number of dirty pages, number of
> pages marked for writeback, number of (recently) evicted pages, etc. in
> a given range.
>
> NAME
> cachestat - query the page cache statistics of a file.
>
> SYNOPSIS
> #include <sys/mman.h>
>
> struct cachestat {
> __u64 nr_cache;
> __u64 nr_dirty;
> __u64 nr_writeback;
> __u64 nr_evicted;
> __u64 nr_recently_evicted;
> };
>
> int cachestat(unsigned int fd, off_t off, size_t len,
> size_t cstat_size, struct cachestat *cstat,
> unsigned int flags);
>
> DESCRIPTION
> cachestat() queries the number of cached pages, number of dirty
> pages, number of pages marked for writeback, number of evicted
> pages, number of recently evicted pages, in the bytes range given by
> `off` and `len`.
>
> An evicted page is a page that is previously in the page cache but
> has been evicted since. A page is recently evicted if its last
> eviction was recent enough that its reentry to the cache would
> indicate that it is actively being used by the system, and that
> there is memory pressure on the system.
>
> These values are returned in a cachestat struct, whose address is
> given by the `cstat` argument.
>
> The `off` and `len` arguments must be non-negative integers. If
> `len` > 0, the queried range is [`off`, `off` + `len`]. If `len` ==
> 0, we will query in the range from `off` to the end of the file.
>
> `cstat_size` allows users to obtain partial results. The syscall
> will copy the first `csstat_size` bytes to the specified userspace
> memory. `cstat_size` must be a non-negative value that is no larger
> than the current size of the cachestat struct.
>
> The `flags` argument is unused for now, but is included for future
> extensibility. User should pass 0 (i.e no flag specified).
>
> RETURN VALUE
> On success, cachestat returns 0. On error, -1 is returned, and errno
> is set to indicate the error.
>
> ERRORS
> EFAULT cstat points to an invalid address.
>
> EINVAL invalid `cstat_size` or `flags`
>
> EBADF invalid file descriptor.
>
> Signed-off-by: Nhat Pham <nphamcs@...il.com>
> ---
> arch/alpha/kernel/syscalls/syscall.tbl | 1 +
> arch/arm/tools/syscall.tbl | 1 +
> arch/ia64/kernel/syscalls/syscall.tbl | 1 +
> arch/m68k/kernel/syscalls/syscall.tbl | 1 +
> arch/microblaze/kernel/syscalls/syscall.tbl | 1 +
> arch/parisc/kernel/syscalls/syscall.tbl | 1 +
> arch/powerpc/kernel/syscalls/syscall.tbl | 1 +
> arch/s390/kernel/syscalls/syscall.tbl | 1 +
> arch/sh/kernel/syscalls/syscall.tbl | 1 +
> arch/sparc/kernel/syscalls/syscall.tbl | 1 +
> arch/x86/entry/syscalls/syscall_32.tbl | 1 +
> arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> arch/xtensa/kernel/syscalls/syscall.tbl | 1 +
> include/linux/fs.h | 3 +
> include/linux/syscalls.h | 3 +
> include/uapi/asm-generic/unistd.h | 5 +-
> include/uapi/linux/mman.h | 9 ++
> init/Kconfig | 10 ++
> kernel/sys_ni.c | 1 +
> mm/filemap.c | 143 ++++++++++++++++++++
> 20 files changed, 186 insertions(+), 1 deletion(-)
>
...
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 08341616ae7a..d70d47b20700 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
...
> @@ -3949,3 +3953,142 @@ bool filemap_release_folio(struct folio *folio, gfp_t gfp)
> return try_to_free_buffers(folio);
> }
> EXPORT_SYMBOL(filemap_release_folio);
...
> +#ifdef CONFIG_CACHESTAT_SYSCALL
> +/*
> + * The cachestat(5) system call.
> + *
> + * cachestat() returns the page cache statistics of a file in the
> + * bytes range specified by `off` and `len`: number of cached pages,
> + * number of dirty pages, number of pages marked for writeback,
> + * number of evicted pages, and number of recently evicted pages.
> + *
> + * An evicted page is a page that is previously in the page cache
> + * but has been evicted since. A page is recently evicted if its last
> + * eviction was recent enough that its reentry to the cache would
> + * indicate that it is actively being used by the system, and that
> + * there is memory pressure on the system.
> + *
> + * `off` and `len` must be non-negative integers. If `len` > 0,
> + * the queried range is [`off`, `off` + `len`]. If `len` == 0,
> + * we will query in the range from `off` to the end of the file.
> + *
> + * `cstat_size` allows users to obtain partial results. The syscall
> + * will copy the first `csstat_size` bytes to the specified userspace
> + * memory. It also makes the cachestat struct extensible - new fields
> + * can be added in the future without breaking existing usage.
> + * `cstat_size` must be a non-negative value that is no larger than
> + * the current size of the cachestat struct.
> + *
> + * The `flags` argument is unused for now, but is included for future
> + * extensibility. User should pass 0 (i.e no flag specified).
> + *
> + * Because the status of a page can change after cachestat() checks it
> + * but before it returns to the application, the returned values may
> + * contain stale information.
> + *
> + * return values:
> + * zero - success
> + * -EFAULT - cstat points to an illegal address
> + * -EINVAL - invalid arguments
> + * -EBADF - invalid file descriptor
> + */
> +SYSCALL_DEFINE6(cachestat, unsigned int, fd, off_t, off, size_t, len,
> + size_t, cstat_size, struct cachestat __user *, cstat,
> + unsigned int, flags)
> +{
> + struct fd f = fdget(fd);
> + struct address_space *mapping;
> + struct cachestat cs;
> + pgoff_t first_index = off >> PAGE_SHIFT;
> + pgoff_t last_index =
> + len == 0 ? ULONG_MAX : (off + len - 1) >> PAGE_SHIFT;
> +
> + if (off < 0 || cstat_size > sizeof(struct cachestat) || flags != 0)
> + return -EINVAL;
> +
> + if (!f.file)
> + return -EBADF;
> +
It looks like we miss an fdput() before returning via the above error
checks.
The only other thing that stands out as a bit odd to me is the
cstat_size check and associated ability to return a partial cachestat
struct. Do other syscalls do anything like that? I'd think we'd want to
ensure we always at least return a fully populated cachestat struct,
even if it happened to be an old/compat version if the size does ever
increase. Hm?
Brian
> + memset(&cs, 0, sizeof(struct cachestat));
> + mapping = f.file->f_mapping;
> + filemap_cachestat(mapping, first_index, last_index, &cs);
> + fdput(f);
> +
> + if (copy_to_user(cstat, &cs, cstat_size))
> + return -EFAULT;
> +
> + return 0;
> +}
> +#endif /* CONFIG_CACHESTAT_SYSCALL */
> --
> 2.30.2
>
Powered by blists - more mailing lists