linux-kernel - Re: [PATCH] libnvdimm, pmem: clarify the write+clear

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] [day] [month] [year] [list]

Message-ID: <1460739500.3012.4.camel@intel.com>
Date:	Fri, 15 Apr 2016 16:58:20 +0000
From:	"Verma, Vishal L" <vishal.l.verma@...el.com>
To:	"Williams, Dan J" <dan.j.williams@...el.com>,
	"linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"jmoyer@...hat.com" <jmoyer@...hat.com>
Subject: Re: [PATCH] libnvdimm, pmem: clarify the write+clear_poison+write
 flow

On Thu, 2016-04-14 at 19:40 -0700, Dan Williams wrote:
> The ACPI specification does not specify the state of data after a
> clear
> poison operation.  Potential future libnvdimm bus implementations for
> other architectures also might not specify or disagree on the state
> of
> data after clear poison.  Clarify why we write twice.
> 
> Reported-by: Jeff Moyer <jmoyer@...hat.com>
> Reported-by: Vishal Verma <vishal.l.verma@...el.com>
> Signed-off-by: Dan Williams <dan.j.williams@...el.com>
> ---
>  drivers/nvdimm/pmem.c |   14 ++++++++++++++
>  1 file changed, 14 insertions(+)

Looks good, thanks!

Reviewed-by: Vishal Verma <vishal.l.verma@...el.com>

> 
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index c6befaa9c708..d9a0dbc2d023 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -86,6 +86,20 @@ static int pmem_do_bvec(struct pmem_device *pmem,
> struct page *page,
>  			flush_dcache_page(page);
>  		}
>  	} else {
> +		/*
> +		 * Note that we write the data both before and after
> +		 * clearing poison.  The write before clear poison
> +		 * handles situations where the latest written data
> is
> +		 * preserved and the clear poison operation simply
> marks
> +		 * the address range as valid without changing the
> data.
> +		 * In this case application software can assume that
> an
> +		 * interrupted write will either return the new good
> +		 * data or an error.
> +		 *
> +		 * However, if pmem_clear_poison() leaves the data
> in an
> +		 * indeterminate state we need to perform the write
> +		 * after clear poison.
> +		 */
>  		flush_dcache_page(page);
>  		memcpy_to_pmem(pmem_addr, mem + off, len);
>  		if (unlikely(bad_pmem)) {
>