[makedumpfile PATCH RFC v0.1] Implemented the --fill-excluded-pages=<value> feature

[makedumpfile PATCH RFC v0.1] Implemented the --fill-excluded-pages=<value> feature

[Eric DeVolder]

When a page is excluded by any of the existing dump levels,
that page may still be written to the ELF dump file, depending
upon the PFN_EXCLUDED mechanism.

The PFN_EXCLUDED mechanism looks for N consecutive "not
dumpable" pages, and if found, the current ELF segment is
closed out and a new ELF segment started, at the next dumpable
page. Otherwise, if the PFN_EXCLUDED criteria is not meet (that
is, there is a mix of dumpable and not dumpable pages, but not
N consecutive not dumpable pages) all pages are written to the
dump file.

This patch implements a mechanism for those "not dumpable" pages
that are written to the ELF dump file to fill those pages with
constant data, rather than the original data. In other words,
the dump file still contains the page, but its data is wiped.

The motivation for doing this is to protect real user data from
"leaking" through to a dump file when that data was asked to be
omitted. This is especially important for effort I currently am
working on to allow further refinement of what is allowed to be
dumped, all in an effort to protect user (customer) data.

The patch is simple enough, however, it causes problems with
crash; crash is unable to load the resulting ELF dump file.
For example, I do the following as a test scenario for this
change:

- Obtain a non-filtered dump file (eg. dump level 0, no -d option,
  or straight copy of /proc/vmcore)
- Run vmcore through 'crash' to ensure loads ok, test with
  commands like: ps, files, etc.
  % crash vmlinux vmcore
- Apply this patch and rebuild makedumpfile
- Run vmcore through makedumpfile *without* --fill-excluded-pages
  and with filtering to ensure no uintended side effects of patch:
  % ./makedumpfile -E -d31 -x vmlinux vmcore newvmcore
- Run new vmcore through crash to ensure still loads ok, test
  with commands like: ps, files, etc.
  % crash vmlinux newvmcore
- Run vmcore through makedumpfile *with* --fill-excluded-pages
  and with filtering to check side effects of patch:
  % ./makedumpfile -E -d31 --fill-excluded-pages=0 -x vmlinux vmcore newvmcore2
- Run new vmcore through crash to ensure still loads ok, test
  with commands like: ps, files, etc.
  % crash vmlinux newvmcore2

But crash yields errors like:
  [...]
  This GDB was configured as "x86_64-unknown-linux-gnu"...

  crash: cannot determine thread return address
  please wait... (gathering kmem slab cache data)
  crash: invalid kernel virtual address: 1c  type: "kmem_cache objsize/object_size"

If the patch is correct/accurate, then that may mean that crash
is using data which it should not be.

The more likely scenario is that the patch is not correct/accurate,
and I'm corrupting the dump file.

Please provide feedback!!
---
 makedumpfile.c | 28 +++++++++++++++++++++-------
 makedumpfile.h |  3 +++
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index e69b6df..3f9816a 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -7102,7 +7102,7 @@ out:
 
 int
 write_elf_load_segment(struct cache_data *cd_page, unsigned long long paddr,
-		       off_t off_memory, long long size)
+		       off_t off_memory, long long size, struct cycle *cycle)
 {
 	long page_size = info->page_size;
 	long long bufsz_write;
@@ -7126,10 +7126,18 @@ write_elf_load_segment(struct cache_data *cd_page, unsigned long long paddr,
 		else
 			bufsz_write = size;
 
-		if (read(info->fd_memory, buf, bufsz_write) != bufsz_write) {
-			ERRMSG("Can't read the dump memory(%s). %s\n",
-			    info->name_memory, strerror(errno));
-			return FALSE;
+		if (info->flag_fill_excluded_pages && !is_dumpable(info->bitmap2, paddr_to_pfn(paddr), cycle)) {
+			unsigned k;
+			unsigned long *p = (unsigned long *)buf;
+			for (k = 0; k < info->page_size; k += sizeof(unsigned long)) {
+				*p++ = info->fill_excluded_pages_value;
+			}
+		} else {
+			if (read(info->fd_memory, buf, bufsz_write) != bufsz_write) {
+				ERRMSG("Can't read the dump memory(%s). %s\n",
+				    info->name_memory, strerror(errno));
+				return FALSE;
+			}
 		}
 		filter_data_buffer((unsigned char *)buf, paddr, bufsz_write);
 		paddr += bufsz_write;
@@ -7396,7 +7404,7 @@ write_elf_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page)
 				 */
 				if (load.p_filesz)
 					if (!write_elf_load_segment(cd_page, paddr,
-								    off_memory, load.p_filesz))
+								    off_memory, load.p_filesz, &cycle))
 						return FALSE;
 
 				load.p_paddr += load.p_memsz;
@@ -7438,7 +7446,7 @@ write_elf_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page)
 		 */
 		if (load.p_filesz)
 			if (!write_elf_load_segment(cd_page, paddr,
-						    off_memory, load.p_filesz))
+						    off_memory, load.p_filesz, &cycle))
 				return FALSE;
 
 		off_seg_load += load.p_filesz;
@@ -11007,6 +11015,7 @@ static struct option longopts[] = {
 	{"splitblock-size", required_argument, NULL, OPT_SPLITBLOCK_SIZE},
 	{"work-dir", required_argument, NULL, OPT_WORKING_DIR},
 	{"num-threads", required_argument, NULL, OPT_NUM_THREADS},
+	{"fill-excluded-pages", optional_argument, NULL, OPT_FILL_EXCLUDED_PAGES},
 	{0, 0, 0, 0}
 };
 
@@ -11163,6 +11172,11 @@ main(int argc, char *argv[])
 		case OPT_NUM_THREADS:
 			info->num_threads = MAX(atoi(optarg), 0);
 			break;
+		case OPT_FILL_EXCLUDED_PAGES:
+			info->flag_fill_excluded_pages = 1;
+			if (optarg)
+				info->fill_excluded_pages_value = strtoul(optarg, NULL, 0);
+			break;
 		case '?':
 			MSG("Commandline parameter is invalid.\n");
 			MSG("Try `makedumpfile --help' for more information.\n");
diff --git a/makedumpfile.h b/makedumpfile.h
index e32e567..4069672 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -1135,6 +1135,8 @@ struct DumpInfo {
 	int		flag_vmemmap;        /* kernel supports vmemmap address space */
 	int		flag_excludevm;      /* -e - excluding unused vmemmap pages */
 	int		flag_use_count;      /* _refcount is named _count in struct page */
+	int		flag_fill_excluded_pages;    /* if pages are excluded but still dumped (see PFN_EXCLUDED operation), replace with page with fill */
+	unsigned long	fill_excluded_pages_value; /* fill value for excluded pages */
 	unsigned long	vaddr_for_vtop;      /* virtual address for debugging */
 	long		page_size;           /* size of page */
 	long		page_shift;
@@ -2143,6 +2145,7 @@ struct elf_prstatus {
 #define OPT_SPLITBLOCK_SIZE	OPT_START+14
 #define OPT_WORKING_DIR         OPT_START+15
 #define OPT_NUM_THREADS	OPT_START+16
+#define OPT_FILL_EXCLUDED_PAGES	OPT_START+17
 
 /*
  * Function Prototype.
-- 
2.7.4


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

Re: [makedumpfile PATCH RFC v0.1] Implemented the --fill-excluded-pages=<value> feature

[Eric DeVolder]

I have fixed the problem with the patch below.
I will re-submit soon.
eric

On 07/18/2017 09:19 AM, Eric DeVolder wrote:
> When a page is excluded by any of the existing dump levels,
> that page may still be written to the ELF dump file, depending
> upon the PFN_EXCLUDED mechanism.
> 
> The PFN_EXCLUDED mechanism looks for N consecutive "not
> dumpable" pages, and if found, the current ELF segment is
> closed out and a new ELF segment started, at the next dumpable
> page. Otherwise, if the PFN_EXCLUDED criteria is not meet (that
> is, there is a mix of dumpable and not dumpable pages, but not
> N consecutive not dumpable pages) all pages are written to the
> dump file.
> 
> This patch implements a mechanism for those "not dumpable" pages
> that are written to the ELF dump file to fill those pages with
> constant data, rather than the original data. In other words,
> the dump file still contains the page, but its data is wiped.
> 
> The motivation for doing this is to protect real user data from
> "leaking" through to a dump file when that data was asked to be
> omitted. This is especially important for effort I currently am
> working on to allow further refinement of what is allowed to be
> dumped, all in an effort to protect user (customer) data.
> 
> The patch is simple enough, however, it causes problems with
> crash; crash is unable to load the resulting ELF dump file.
> For example, I do the following as a test scenario for this
> change:
> 
> - Obtain a non-filtered dump file (eg. dump level 0, no -d option,
>    or straight copy of /proc/vmcore)
> - Run vmcore through 'crash' to ensure loads ok, test with
>    commands like: ps, files, etc.
>    % crash vmlinux vmcore
> - Apply this patch and rebuild makedumpfile
> - Run vmcore through makedumpfile *without* --fill-excluded-pages
>    and with filtering to ensure no uintended side effects of patch:
>    % ./makedumpfile -E -d31 -x vmlinux vmcore newvmcore
> - Run new vmcore through crash to ensure still loads ok, test
>    with commands like: ps, files, etc.
>    % crash vmlinux newvmcore
> - Run vmcore through makedumpfile *with* --fill-excluded-pages
>    and with filtering to check side effects of patch:
>    % ./makedumpfile -E -d31 --fill-excluded-pages=0 -x vmlinux vmcore newvmcore2
> - Run new vmcore through crash to ensure still loads ok, test
>    with commands like: ps, files, etc.
>    % crash vmlinux newvmcore2
> 
> But crash yields errors like:
>    [...]
>    This GDB was configured as "x86_64-unknown-linux-gnu"...
> 
>    crash: cannot determine thread return address
>    please wait... (gathering kmem slab cache data)
>    crash: invalid kernel virtual address: 1c  type: "kmem_cache objsize/object_size"
> 
> If the patch is correct/accurate, then that may mean that crash
> is using data which it should not be.
> 
> The more likely scenario is that the patch is not correct/accurate,
> and I'm corrupting the dump file.
> 
> Please provide feedback!!
> ---
>   makedumpfile.c | 28 +++++++++++++++++++++-------
>   makedumpfile.h |  3 +++
>   2 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index e69b6df..3f9816a 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -7102,7 +7102,7 @@ out:
>   
>   int
>   write_elf_load_segment(struct cache_data *cd_page, unsigned long long paddr,
> -		       off_t off_memory, long long size)
> +		       off_t off_memory, long long size, struct cycle *cycle)
>   {
>   	long page_size = info->page_size;
>   	long long bufsz_write;
> @@ -7126,10 +7126,18 @@ write_elf_load_segment(struct cache_data *cd_page, unsigned long long paddr,
>   		else
>   			bufsz_write = size;
>   
> -		if (read(info->fd_memory, buf, bufsz_write) != bufsz_write) {
> -			ERRMSG("Can't read the dump memory(%s). %s\n",
> -			    info->name_memory, strerror(errno));
> -			return FALSE;
> +		if (info->flag_fill_excluded_pages && !is_dumpable(info->bitmap2, paddr_to_pfn(paddr), cycle)) {
> +			unsigned k;
> +			unsigned long *p = (unsigned long *)buf;
> +			for (k = 0; k < info->page_size; k += sizeof(unsigned long)) {
> +				*p++ = info->fill_excluded_pages_value;
> +			}
> +		} else {
> +			if (read(info->fd_memory, buf, bufsz_write) != bufsz_write) {
> +				ERRMSG("Can't read the dump memory(%s). %s\n",
> +				    info->name_memory, strerror(errno));
> +				return FALSE;
> +			}
>   		}
>   		filter_data_buffer((unsigned char *)buf, paddr, bufsz_write);
>   		paddr += bufsz_write;
> @@ -7396,7 +7404,7 @@ write_elf_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page)
>   				 */
>   				if (load.p_filesz)
>   					if (!write_elf_load_segment(cd_page, paddr,
> -								    off_memory, load.p_filesz))
> +								    off_memory, load.p_filesz, &cycle))
>   						return FALSE;
>   
>   				load.p_paddr += load.p_memsz;
> @@ -7438,7 +7446,7 @@ write_elf_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page)
>   		 */
>   		if (load.p_filesz)
>   			if (!write_elf_load_segment(cd_page, paddr,
> -						    off_memory, load.p_filesz))
> +						    off_memory, load.p_filesz, &cycle))
>   				return FALSE;
>   
>   		off_seg_load += load.p_filesz;
> @@ -11007,6 +11015,7 @@ static struct option longopts[] = {
>   	{"splitblock-size", required_argument, NULL, OPT_SPLITBLOCK_SIZE},
>   	{"work-dir", required_argument, NULL, OPT_WORKING_DIR},
>   	{"num-threads", required_argument, NULL, OPT_NUM_THREADS},
> +	{"fill-excluded-pages", optional_argument, NULL, OPT_FILL_EXCLUDED_PAGES},
>   	{0, 0, 0, 0}
>   };
>   
> @@ -11163,6 +11172,11 @@ main(int argc, char *argv[])
>   		case OPT_NUM_THREADS:
>   			info->num_threads = MAX(atoi(optarg), 0);
>   			break;
> +		case OPT_FILL_EXCLUDED_PAGES:
> +			info->flag_fill_excluded_pages = 1;
> +			if (optarg)
> +				info->fill_excluded_pages_value = strtoul(optarg, NULL, 0);
> +			break;
>   		case '?':
>   			MSG("Commandline parameter is invalid.\n");
>   			MSG("Try `makedumpfile --help' for more information.\n");
> diff --git a/makedumpfile.h b/makedumpfile.h
> index e32e567..4069672 100644
> --- a/makedumpfile.h
> +++ b/makedumpfile.h
> @@ -1135,6 +1135,8 @@ struct DumpInfo {
>   	int		flag_vmemmap;        /* kernel supports vmemmap address space */
>   	int		flag_excludevm;      /* -e - excluding unused vmemmap pages */
>   	int		flag_use_count;      /* _refcount is named _count in struct page */
> +	int		flag_fill_excluded_pages;    /* if pages are excluded but still dumped (see PFN_EXCLUDED operation), replace with page with fill */
> +	unsigned long	fill_excluded_pages_value; /* fill value for excluded pages */
>   	unsigned long	vaddr_for_vtop;      /* virtual address for debugging */
>   	long		page_size;           /* size of page */
>   	long		page_shift;
> @@ -2143,6 +2145,7 @@ struct elf_prstatus {
>   #define OPT_SPLITBLOCK_SIZE	OPT_START+14
>   #define OPT_WORKING_DIR         OPT_START+15
>   #define OPT_NUM_THREADS	OPT_START+16
> +#define OPT_FILL_EXCLUDED_PAGES	OPT_START+17
>   
>   /*
>    * Function Prototype.
> 

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

Re: [makedumpfile PATCH RFC v0.1] Implemented the --fill-excluded-pages=<value> feature

[Eric DeVolder]

On 07/20/2017 08:20 AM, Dave Anderson wrote:
> 
> 
> ----- Original Message -----
> 
>> When a page is excluded by any of the existing dump levels,
>> that page may still be written to the ELF dump file, depending
>> upon the PFN_EXCLUDED mechanism.
>>
>> The PFN_EXCLUDED mechanism looks for N consecutive "not
>> dumpable" pages, and if found, the current ELF segment is
>> closed out and a new ELF segment started, at the next dumpable
>> page. Otherwise, if the PFN_EXCLUDED criteria is not meet (that
>> is, there is a mix of dumpable and not dumpable pages, but not
>> N consecutive not dumpable pages) all pages are written to the
>> dump file.
>>
>> This patch implements a mechanism for those "not dumpable" pages
>> that are written to the ELF dump file to fill those pages with
>> constant data, rather than the original data. In other words,
>> the dump file still contains the page, but its data is wiped.
>>
>> The motivation for doing this is to protect real user data from
>> "leaking" through to a dump file when that data was asked to be
>> omitted. This is especially important for effort I currently am
>> working on to allow further refinement of what is allowed to be
>> dumped, all in an effort to protect user (customer) data.
>>
>> The patch is simple enough, however, it causes problems with
>> crash; crash is unable to load the resulting ELF dump file.
>> For example, I do the following as a test scenario for this
>> change:
>>
>> - Obtain a non-filtered dump file (eg. dump level 0, no -d option,
>>    or straight copy of /proc/vmcore)
>> - Run vmcore through 'crash' to ensure loads ok, test with
>>    commands like: ps, files, etc.
>>    % crash vmlinux vmcore
>> - Apply this patch and rebuild makedumpfile
>> - Run vmcore through makedumpfile *without* --fill-excluded-pages
>>    and with filtering to ensure no uintended side effects of patch:
>>    % ./makedumpfile -E -d31 -x vmlinux vmcore newvmcore
>> - Run new vmcore through crash to ensure still loads ok, test
>>    with commands like: ps, files, etc.
>>    % crash vmlinux newvmcore
>> - Run vmcore through makedumpfile *with* --fill-excluded-pages
>>    and with filtering to check side effects of patch:
>>    % ./makedumpfile -E -d31 --fill-excluded-pages=0 -x vmlinux vmcore
>>    newvmcore2
>> - Run new vmcore through crash to ensure still loads ok, test
>>    with commands like: ps, files, etc.
>>    % crash vmlinux newvmcore2
>>
>> But crash yields errors like:
>>    [...]
>>    This GDB was configured as "x86_64-unknown-linux-gnu"...
>>
>>    crash: cannot determine thread return address
>>    please wait... (gathering kmem slab cache data)
>>    crash: invalid kernel virtual address: 1c  type: "kmem_cache
>>    objsize/object_size"
>>
>> If the patch is correct/accurate, then that may mean that crash
>> is using data which it should not be.
> 
> Why would the crash utility be "using data which it should not be"
> if your patch is applied?
> 
> The two error messages above come from attempting to read memory
> from the kernel text region (the "thread return" message), and then
> the kmem_cache.object_size field of the kernel's kmem_cache data structure
> pointed to by its "kmem_cache" pointer.  It looks like the patch is
> causing bogus data to be returned for a given physical address >
> Dave
> 

Indeed, the patch was incorrect and was causing bogus data to be 
returned. I've corrected the patch and will re-post soon.

Eric
> 
>>
>> The more likely scenario is that the patch is not correct/accurate,
>> and I'm corrupting the dump file.
>>
>> Please provide feedback!!
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
> 

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

[makedumpfile PATCH RFC v0.1] Implemented the --fill-excluded-pages=<value> feature

[Dave Anderson]

----- Original Message -----

> When a page is excluded by any of the existing dump levels,
> that page may still be written to the ELF dump file, depending
> upon the PFN_EXCLUDED mechanism.
> 
> The PFN_EXCLUDED mechanism looks for N consecutive "not
> dumpable" pages, and if found, the current ELF segment is
> closed out and a new ELF segment started, at the next dumpable
> page. Otherwise, if the PFN_EXCLUDED criteria is not meet (that
> is, there is a mix of dumpable and not dumpable pages, but not
> N consecutive not dumpable pages) all pages are written to the
> dump file.
> 
> This patch implements a mechanism for those "not dumpable" pages
> that are written to the ELF dump file to fill those pages with
> constant data, rather than the original data. In other words,
> the dump file still contains the page, but its data is wiped.
> 
> The motivation for doing this is to protect real user data from
> "leaking" through to a dump file when that data was asked to be
> omitted. This is especially important for effort I currently am
> working on to allow further refinement of what is allowed to be
> dumped, all in an effort to protect user (customer) data.
> 
> The patch is simple enough, however, it causes problems with
> crash; crash is unable to load the resulting ELF dump file.
> For example, I do the following as a test scenario for this
> change:
> 
> - Obtain a non-filtered dump file (eg. dump level 0, no -d option,
>   or straight copy of /proc/vmcore)
> - Run vmcore through 'crash' to ensure loads ok, test with
>   commands like: ps, files, etc.
>   % crash vmlinux vmcore
> - Apply this patch and rebuild makedumpfile
> - Run vmcore through makedumpfile *without* --fill-excluded-pages
>   and with filtering to ensure no uintended side effects of patch:
>   % ./makedumpfile -E -d31 -x vmlinux vmcore newvmcore
> - Run new vmcore through crash to ensure still loads ok, test
>   with commands like: ps, files, etc.
>   % crash vmlinux newvmcore
> - Run vmcore through makedumpfile *with* --fill-excluded-pages
>   and with filtering to check side effects of patch:
>   % ./makedumpfile -E -d31 --fill-excluded-pages=0 -x vmlinux vmcore
>   newvmcore2
> - Run new vmcore through crash to ensure still loads ok, test
>   with commands like: ps, files, etc.
>   % crash vmlinux newvmcore2
> 
> But crash yields errors like:
>   [...]
>   This GDB was configured as "x86_64-unknown-linux-gnu"...
> 
>   crash: cannot determine thread return address
>   please wait... (gathering kmem slab cache data)
>   crash: invalid kernel virtual address: 1c  type: "kmem_cache
>   objsize/object_size"
> 
> If the patch is correct/accurate, then that may mean that crash
> is using data which it should not be.

Why would the crash utility be "using data which it should not be"
if your patch is applied?

The two error messages above come from attempting to read memory
from the kernel text region (the "thread return" message), and then
the kmem_cache.object_size field of the kernel's kmem_cache data structure
pointed to by its "kmem_cache" pointer.  It looks like the patch is
causing bogus data to be returned for a given physical address.

Dave


> 
> The more likely scenario is that the patch is not correct/accurate,
> and I'm corrupting the dump file.
> 
> Please provide feedback!!

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