Re: [PATCH 11/11] sysctl: treewide: constify the ctl_table argument of handlers

2024-03-27 Thread Dave Chinner
   int write,
>  void *buffer, size_t *lenp, loff_t *ppos)
>  {
>   int ret;

And this.

> @@ -474,8 +475,10 @@ int perf_event_max_sample_rate_handler(struct ctl_table 
> *table, int write,
>  
>  int sysctl_perf_cpu_time_max_percent __read_mostly = 
> DEFAULT_CPU_TIME_MAX_PERCENT;
>  
> -int perf_cpu_time_max_percent_handler(struct ctl_table *table, int write,
> - void *buffer, size_t *lenp, loff_t *ppos)
> +int perf_cpu_time_max_percent_handler(const struct ctl_table *table,
> +   int write,
> +   void *buffer, size_t *lenp,
> +   loff_t *ppos)
>  {
>   int ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
>  

And this.

> diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> index b2fc2727d654..003f0f5cb111 100644
> --- a/kernel/hung_task.c
> +++ b/kernel/hung_task.c
> @@ -239,9 +239,10 @@ static long hung_timeout_jiffies(unsigned long 
> last_checked,
>  /*
>   * Process updating of timeout sysctl
>   */
> -static int proc_dohung_task_timeout_secs(struct ctl_table *table, int write,
> -   void *buffer,
> -   size_t *lenp, loff_t *ppos)
> +static int proc_dohung_task_timeout_secs(const struct ctl_table *table,
> +  int write,
> +  void *buffer,
> +  size_t *lenp, loff_t *ppos)
>  {
>   int ret;
>  

And this.

> diff --git a/kernel/latencytop.c b/kernel/latencytop.c
> index 781249098cb6..0a5c22b19821 100644
> --- a/kernel/latencytop.c
> +++ b/kernel/latencytop.c
> @@ -65,8 +65,9 @@ static struct latency_record latency_record[MAXLR];
>  int latencytop_enabled;
>  
>  #ifdef CONFIG_SYSCTL
> -static int sysctl_latencytop(struct ctl_table *table, int write, void 
> *buffer,
> -     size_t *lenp, loff_t *ppos)
> +static int sysctl_latencytop(const struct ctl_table *table, int write,
> +  void *buffer,
> +  size_t *lenp, loff_t *ppos)
>  {
>   int err;
>  

And this.

I could go on, but there are so many examples of this in the patch
that I think that it needs to be toosed away and regenerated in a
way that doesn't trash the existing function parameter formatting.

-Dave.
-- 
Dave Chinner
da...@fromorbit.com

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [RFC 05/18] pkernfs: add file mmap callback

2024-02-05 Thread Dave Chinner
On Mon, Feb 05, 2024 at 12:01:50PM +, James Gowans wrote:
> Make the file data useable to userspace by adding mmap. That's all that
> QEMU needs for guest RAM, so that's all be bother implementing for now.
> 
> When mmaping the file the VMA is marked as PFNMAP to indicate that there
> are no struct pages for the memory in this VMA. Remap_pfn_range() is
> used to actually populate the page tables. All PTEs are pre-faulted into
> the pgtables at mmap time so that the pgtables are useable when this
> virtual address range is given to VFIO's MAP_DMA.

And so what happens when this file is truncated whilst it is mmap()d
by an application? Ain't that just a great big UAF waiting to be
exploited?

-Dave.
-- 
Dave Chinner
da...@fromorbit.com

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH 6/6] fs: add automatic kernel fs freeze / thaw and remove kthread freezing

2023-05-08 Thread Dave Chinner
On Sun, May 07, 2023 at 06:17:17PM -0700, Luis Chamberlain wrote:
> Add support to automatically handle freezing and thawing filesystems
> during the kernel's suspend/resume cycle.
> 
> This is needed so that we properly really stop IO in flight without
> races after userspace has been frozen. Without this we rely on
> kthread freezing and its semantics are loose and error prone.
> For instance, even though a kthread may use try_to_freeze() and end
> up being frozen we have no way of being sure that everything that
> has been spawned asynchronously from it (such as timers) have also
> been stopped as well.
> 
> A long term advantage of also adding filesystem freeze / thawing
> supporting during suspend / hibernation is that long term we may
> be able to eventually drop the kernel's thread freezing completely
> as it was originally added to stop disk IO in flight as we hibernate
> or suspend.
> 
> This does not remove the superfluous freezer calls on all filesystems.
> Each filesystem must remove all the kthread freezer stuff and peg
> the fs_type flags as supporting auto-freezing with the FS_AUTOFREEZE
> flag.
> 
> Subsequent patches remove the kthread freezer usage from each
> filesystem, one at a time to make all this work bisectable.
> Once all filesystems remove the usage of the kthread freezer we
> can remove the FS_AUTOFREEZE flag.
> 
> Reviewed-by: Jan Kara 
> Signed-off-by: Luis Chamberlain 
> ---
>  fs/super.c | 50 ++
>  include/linux/fs.h | 14 
>  kernel/power/process.c | 15 -
>  3 files changed, 78 insertions(+), 1 deletion(-)

.

> diff --git a/kernel/power/process.c b/kernel/power/process.c
> index cae81a87cc91..7ca7688f0b5d 100644
> --- a/kernel/power/process.c
> +++ b/kernel/power/process.c
> @@ -140,6 +140,16 @@ int freeze_processes(void)
>  
>   BUG_ON(in_atomic());
>  
> + pr_info("Freezing filesystems ... ");
> + error = iterate_supers_reverse_excl(fs_suspend_freeze_sb, NULL);
> + if (error) {
> + pr_cont("failed\n");
> + iterate_supers_excl(fs_suspend_thaw_sb, NULL);
> + thaw_processes();
> + return error;

That looks wrong. i.e. if the sb iteration fails to freeze a
filesystem (for whatever reason) then every userspace frozen
filesystem will be thawed by this call, right? i.e. it will thaw
more than just the filesystems frozen by the suspend freeze
iteration before it failed.

Don't we only want to thaw the superblocks we froze before the
failure occurred? i.e. the "undo" iteration needs to start from the
last superblock we successfully froze and then only walk to the tail
of the list we started from?

> + }
> + pr_cont("done.\n");
> +
>   /*
>* Now that the whole userspace is frozen we need to disable
>* the OOM killer to disallow any further interference with
> @@ -149,8 +159,10 @@ int freeze_processes(void)
>   if (!error && 
> !oom_killer_disable(msecs_to_jiffies(freeze_timeout_msecs)))
>   error = -EBUSY;
>  
> - if (error)
> + if (error) {
> + iterate_supers_excl(fs_suspend_thaw_sb, NULL);
>   thaw_processes();
> +     }

Does this also have the same problem? i.e. if fs_suspend_freeze_sb()
skips over superblocks that are already userspace frozen without any
error, then this will incorrectly thaw those userspace frozen
filesystems.

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec