diff options
| author | Dan Williams <dan.j.williams@intel.com> | 2016-12-16 11:10:31 -0500 |
|---|---|---|
| committer | Dan Williams <dan.j.williams@intel.com> | 2016-12-16 11:10:31 -0500 |
| commit | 868f036fee4b1f934117197fb93461d2c968ffec (patch) | |
| tree | 2ccd2f941f1a6321d15d0c6c948fd527ebeae469 | |
| parent | 9cf8bd529c6ba81402ebf6b7a56307b0787e4f93 (diff) | |
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 0a3f27b9a6a8 "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: 82bf1037f2ca ("libnvdimm: check and clear poison before writing to pmem")
Fixes: 0a3f27b9a6a8 ("libnvdimm, namespace: avoid multiple sector calculations")
Cc: Fabian Frederick <fabf@skynet.be>
Cc: Dave Jiang <dave.jiang@intel.com>
Reported-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
| -rw-r--r-- | drivers/nvdimm/claim.c | 9 | ||||
| -rw-r--r-- | drivers/nvdimm/pmem.c | 21 |
2 files changed, 19 insertions, 11 deletions
diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c index 97d1772774a8..b3323c0697f6 100644 --- a/drivers/nvdimm/claim.c +++ b/drivers/nvdimm/claim.c | |||
| @@ -247,12 +247,13 @@ static int nsio_rw_bytes(struct nd_namespace_common *ndns, | |||
| 247 | long cleared; | 247 | long cleared; |
| 248 | 248 | ||
| 249 | cleared = nvdimm_clear_poison(&ndns->dev, offset, size); | 249 | cleared = nvdimm_clear_poison(&ndns->dev, offset, size); |
| 250 | if (cleared != size) { | 250 | if (cleared < size) |
| 251 | size = cleared; | ||
| 252 | rc = -EIO; | 251 | rc = -EIO; |
| 252 | if (cleared > 0 && cleared / 512) { | ||
| 253 | cleared /= 512; | ||
| 254 | badblocks_clear(&nsio->bb, sector, cleared); | ||
| 253 | } | 255 | } |
| 254 | 256 | invalidate_pmem(nsio->addr + offset, size); | |
| 255 | badblocks_clear(&nsio->bb, sector, cleared >> 9); | ||
| 256 | } else | 257 | } else |
| 257 | rc = -EIO; | 258 | rc = -EIO; |
| 258 | } | 259 | } |
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c index bcc359a4e64d..ecf79fd64517 100644 --- a/drivers/nvdimm/pmem.c +++ b/drivers/nvdimm/pmem.c | |||
| @@ -47,22 +47,29 @@ static struct nd_region *to_region(struct pmem_device *pmem) | |||
| 47 | return to_nd_region(to_dev(pmem)->parent); | 47 | return to_nd_region(to_dev(pmem)->parent); |
| 48 | } | 48 | } |
| 49 | 49 | ||
| 50 | static void pmem_clear_poison(struct pmem_device *pmem, phys_addr_t offset, | 50 | static int pmem_clear_poison(struct pmem_device *pmem, phys_addr_t offset, |
| 51 | unsigned int len) | 51 | unsigned int len) |
| 52 | { | 52 | { |
| 53 | struct device *dev = to_dev(pmem); | 53 | struct device *dev = to_dev(pmem); |
| 54 | sector_t sector, cleared; | 54 | sector_t sector; |
| 55 | long cleared; | ||
| 56 | int rc = 0; | ||
| 55 | 57 | ||
| 56 | sector = (offset - pmem->data_offset) / 512; | 58 | sector = (offset - pmem->data_offset) / 512; |
| 57 | cleared = nvdimm_clear_poison(dev, pmem->phys_addr + offset, len) / 512; | ||
| 58 | 59 | ||
| 59 | if (cleared) { | 60 | cleared = nvdimm_clear_poison(dev, pmem->phys_addr + offset, len); |
| 60 | dev_dbg(dev, "%s: %#llx clear %ld sector%s\n", | 61 | if (cleared < len) |
| 61 | __func__, (unsigned long long) sector, | 62 | rc = -EIO; |
| 62 | cleared, cleared > 1 ? "s" : ""); | 63 | if (cleared > 0 && cleared / 512) { |
| 64 | cleared /= 512; | ||
| 65 | dev_dbg(dev, "%s: %#llx clear %ld sector%s\n", __func__, | ||
| 66 | (unsigned long long) sector, cleared, | ||
| 67 | cleared > 1 ? "s" : ""); | ||
| 63 | badblocks_clear(&pmem->bb, sector, cleared); | 68 | badblocks_clear(&pmem->bb, sector, cleared); |
| 64 | } | 69 | } |
| 65 | invalidate_pmem(pmem->virt_addr + offset, len); | 70 | invalidate_pmem(pmem->virt_addr + offset, len); |
| 71 | |||
| 72 | return rc; | ||
| 66 | } | 73 | } |
| 67 | 74 | ||
| 68 | static void write_pmem(void *pmem_addr, struct page *page, | 75 | static void write_pmem(void *pmem_addr, struct page *page, |
