Commit 868f036f authored by Dan Williams's avatar Dan Williams
Browse files

libnvdimm: fix mishandled nvdimm_clear_poison() return value 



Colin, via static analysis, reports that the length could be negative
from nvdimm_clear_poison() in the error case. There was a similar
problem with commit 0a3f27b9 "libnvdimm, namespace: avoid multiple
sector calculations" that I noticed when merging the for-4.10/libnvdimm
topic branch into libnvdimm-for-next, but I missed this one. Fix both of
them to the following procedure:

* if we clear a block's worth of media, clear that many blocks in
  badblocks

* if we clear less than the requested size of the transfer return an
  error

* always invalidate cache after any non-error / non-zero
  nvdimm_clear_poison result

Fixes: 82bf1037 ("libnvdimm: check and clear poison before writing to pmem")
Fixes: 0a3f27b9 ("libnvdimm, namespace: avoid multiple sector calculations")
Cc: Fabian Frederick <fabf@skynet.be>
Cc: Dave Jiang <dave.jiang@intel.com>
Reported-by: default avatarColin Ian King <colin.king@canonical.com>
Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
parent 9cf8bd52

drivers/nvdimm/claim.c

+5 −4
Original line number Diff line number Diff line
@@ -247,12 +247,13 @@ static int nsio_rw_bytes(struct nd_namespace_common *ndns, 
			long cleared;



			cleared = nvdimm_clear_poison(&ndns->dev, offset, size);

			if (cleared != size) {

				size = cleared;

			if (cleared < size)

				rc = -EIO;

			if (cleared > 0 && cleared / 512) {

				cleared /= 512;

				badblocks_clear(&nsio->bb, sector, cleared);

			}



			badblocks_clear(&nsio->bb, sector, cleared >> 9);

			invalidate_pmem(nsio->addr + offset, size);

		} else

			rc = -EIO;

	}

drivers/nvdimm/pmem.c

+14 −7
Original line number Diff line number Diff line
@@ -47,22 +47,29 @@ static struct nd_region *to_region(struct pmem_device *pmem) 
	return to_nd_region(to_dev(pmem)->parent);

}



static void pmem_clear_poison(struct pmem_device *pmem, phys_addr_t offset,

static int pmem_clear_poison(struct pmem_device *pmem, phys_addr_t offset,

		unsigned int len)

{

	struct device *dev = to_dev(pmem);

	sector_t sector, cleared;

	sector_t sector;

	long cleared;

	int rc = 0;



	sector = (offset - pmem->data_offset) / 512;

	cleared = nvdimm_clear_poison(dev, pmem->phys_addr + offset, len) / 512;



	if (cleared) {

		dev_dbg(dev, "%s: %#llx clear %ld sector%s\n",

				__func__, (unsigned long long) sector,

				cleared, cleared > 1 ? "s" : "");

	cleared = nvdimm_clear_poison(dev, pmem->phys_addr + offset, len);

	if (cleared < len)

		rc = -EIO;

	if (cleared > 0 && cleared / 512) {

		cleared /= 512;

		dev_dbg(dev, "%s: %#llx clear %ld sector%s\n", __func__,

				(unsigned long long) sector, cleared,

				cleared > 1 ? "s" : "");

		badblocks_clear(&pmem->bb, sector, cleared);

	}

	invalidate_pmem(pmem->virt_addr + offset, len);



	return rc;

}



static void write_pmem(void *pmem_addr, struct page *page,

CRA Git | Maintained and supported by SUSTech CRA and CCSE