linux-kernel - Re: [RFC][PATCH 09/11] check files for checkpointability

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20090309173837.GC7561@us.ibm.com>
Date:	Mon, 9 Mar 2009 10:38:37 -0700
From:	Matt Helsley <matthltc@...ibm.com>
To:	Dave Hansen <dave@...ux.vnet.ibm.com>
Cc:	Ingo Molnar <mingo@...e.hu>,
	containers <containers@...ts.linux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Dave Hansen <dave@...ux.vnet.ibm.com>,
	Christoph Hellwig <hch@...radead.org>,
	Alexey Dobriyan <adobriyan@...il.com>
Subject: Re: [RFC][PATCH 09/11] check files for checkpointability

On Thu, Mar 05, 2009 at 08:39:10AM -0800, Dave Hansen wrote:
> 
> Introduce a files_struct counter to indicate whether a particular
> file_struct has ever contained a file which can not be
> checkpointed.  This flag is a one-way trip; once it is set, it may
> not be unset.
> 
> We assume at allocation that a new files_struct is clean and may
> be checkpointed.  However, as soon as it has had its files filled
> from its parent's, we check it for real in __scan_files_for_cr().
> At that point, we mark it if it contained any uncheckpointable
> files.
> 
> We also check each 'struct file' when it is installed in a fd
> slot.  This way, if anyone open()s or managed to dup() an
> unsuppored file, we can catch it.
> 
> Signed-off-by: Dave Hansen <dave@...ux.vnet.ibm.com>
> ---
> 
>  linux-2.6.git-dave/fs/file.c                  |   19 +++++++++++++++++++
>  linux-2.6.git-dave/fs/open.c                  |    5 +++++
>  linux-2.6.git-dave/include/linux/checkpoint.h |   13 +++++++++++++
>  linux-2.6.git-dave/include/linux/fdtable.h    |    3 +++
>  4 files changed, 40 insertions(+)
> 
> diff -puN fs/file.c~242c7afafb1a81c0d9a41085536f2197d81146e7 fs/file.c
> --- linux-2.6.git/fs/file.c~242c7afafb1a81c0d9a41085536f2197d81146e7	2009-03-05 08:37:04.000000000 -0800
> +++ linux-2.6.git-dave/fs/file.c	2009-03-05 08:37:04.000000000 -0800
> @@ -15,6 +15,7 @@
>  #include <linux/file.h>
>  #include <linux/fdtable.h>
>  #include <linux/bitops.h>
> +#include <linux/checkpoint.h>
>  #include <linux/interrupt.h>
>  #include <linux/spinlock.h>
>  #include <linux/rcupdate.h>
> @@ -285,6 +286,20 @@ static int count_open_files(struct fdtab
>  	return i;
>  }
> 
> +static void __scan_files_for_cr(struct files_struct *files)
> +{
> +	int i;
> +
> +	for (i = 0; i < files->fdtab.max_fds; i++) {
> +		struct file *f = fcheck_files(files, i);
> +		if (!f)
> +			continue;
> +		if (cr_file_supported(f))
> +			continue;
> +		files_deny_checkpointing(files);

At this point couldn't we skip the rest of the loop iterations?

Might it also be useful to print a path to f here? So not only would
the log show the location in the kernel source but we'd also get some
idea of which file caused the problem? Of course "f" isn't always
available everywhere we call files_deny_checkpointing()..

> +	}
> +}
> +
>  /*
>   * Allocate a new files structure and copy contents from the
>   * passed in files structure.
> @@ -303,6 +318,9 @@ struct files_struct *dup_fd(struct files
>  		goto out;
> 
>  	atomic_set(&newf->count, 1);
> +#ifdef CONFIG_CHECKPOINT_RESTART
> +	newf->may_checkpoint = 1;
> +#endif
> 
>  	spin_lock_init(&newf->file_lock);
>  	newf->next_fd = 0;
> @@ -396,6 +414,7 @@ struct files_struct *dup_fd(struct files
> 
>  	rcu_assign_pointer(newf->fdt, new_fdt);
> 
> +	__scan_files_for_cr(newf);
>  	return newf;
> 
>  out_release:
> diff -puN fs/open.c~242c7afafb1a81c0d9a41085536f2197d81146e7 fs/open.c
> --- linux-2.6.git/fs/open.c~242c7afafb1a81c0d9a41085536f2197d81146e7	2009-03-05 08:37:04.000000000 -0800
> +++ linux-2.6.git-dave/fs/open.c	2009-03-05 08:37:04.000000000 -0800
> @@ -29,6 +29,7 @@
>  #include <linux/rcupdate.h>
>  #include <linux/audit.h>
>  #include <linux/falloc.h>
> +#include <linux/checkpoint.h>
> 
>  int vfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  {
> @@ -1015,6 +1016,10 @@ void fd_install(unsigned int fd, struct 
>  {
>  	struct files_struct *files = current->files;
>  	struct fdtable *fdt;
> +
> +	if (!cr_file_supported(file))
> +		files_deny_checkpointing(files);
> +
>  	spin_lock(&files->file_lock);
>  	fdt = files_fdtable(files);
>  	BUG_ON(fdt->fd[fd] != NULL);
> diff -puN include/linux/checkpoint.h~242c7afafb1a81c0d9a41085536f2197d81146e7 include/linux/checkpoint.h
> --- linux-2.6.git/include/linux/checkpoint.h~242c7afafb1a81c0d9a41085536f2197d81146e7	2009-03-05 08:37:04.000000000 -0800
> +++ linux-2.6.git-dave/include/linux/checkpoint.h	2009-03-05 08:37:04.000000000 -0800
> @@ -12,6 +12,7 @@
> 
>  #include <linux/path.h>
>  #include <linux/fs.h>
> +#include <linux/fdtable.h>
> 
>  #ifdef CONFIG_CHECKPOINT_RESTART
> 
> @@ -101,11 +102,23 @@ extern int cr_read_files(struct cr_ctx *
> 
>  #define pr_fmt(fmt) "[%d:c/r:%s] " fmt, task_pid_vnr(current), __func__
> 
> +static inline void __files_deny_checkpointing(struct files_struct *files,
> +		char *file, int line)
> +{
> +	if (!test_and_clear_bit(0, &files->may_checkpoint))
> +		return;
> +	printk(KERN_INFO "process performed an action that can not be "
> +			"checkpointed at: %s:%d\n", file, line);
> +}
> +#define files_deny_checkpointing(f)  \
> +	__files_deny_checkpointing(f, __FILE__, __LINE__)
> +

<snip>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/