Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ndctl list::integer overflow? displaying region iset_id #217

Open
HkBkH opened this issue Aug 23, 2022 · 0 comments
Open

ndctl list::integer overflow? displaying region iset_id #217

HkBkH opened this issue Aug 23, 2022 · 0 comments

Comments

@HkBkH
Copy link

HkBkH commented Aug 23, 2022

ndctl v73 seems to suffer from what looks like an integer overflow when displaying region iset_id, here's an example:

# ndctl list -Ru -r 0
{
  "dev":"region0",
  "size":"1518.00 GiB (1629.94 GB)",
  "align":"96.00 MiB (100.66 MB)",
  "available_size":"1518.00 GiB (1629.94 GB)",
  "max_available_extent":"1518.00 GiB (1629.94 GB)",
  "type":"pmem",
  "iset_id":"0x7fffffffffffffff",
  "persistence_domain":"memory_controller"
}

instead of the expected

# ndctl list -Ru -r 0
{
  "dev":"region0",
  "size":"1518.00 GiB (1629.94 GB)",
  "available_size":"1518.00 GiB (1629.94 GB)",
   "max_available_extent":"1518.00 GiB (1629.94 GB)",
   "type":"pmem",
   "iset_id":"0xb6567f4823472ccc",
    "persistence_domain":"memory_controller"
}

git bisection points to the following commit as the culprit behind
this regression

691cd249750b505753680d2a766280698ce25b75 is the first bad commit
commit 691cd249750b505753680d2a766280698ce25b75
Author: Dan Williams <[email protected]>
Date:   Sun Jan 23 16:52:10 2022 -0800

json: Add support for json_object_new_uint64()

Recent versions of json-c add a proper u64 type. However since ndctl still
needs to build against older json-c add build infrastructure to fallback to
s64.

Link: https://lore.kernel.org/r/164298553057.3021641.17232869374733997747.stgit@dwillia2-desk3.amr.corp.intel.com
Signed-off-by: Dan Williams <[email protected]>
Signed-off-by: Vishal Verma <[email protected]>

config.h.meson |  3 +++
cxl/json.c     |  6 +++---
daxctl/json.c  |  2 +-
meson.build    |  6 ++++++
ndctl/dimm.c   |  2 +-
ndctl/json.c   | 10 +++++-----
util/json.c    |  2 +-
util/json.h    | 13 ++++++++++++-
8 files changed, 32 insertions(+), 12 deletions(-)

Cheers,
Hedi.
justinrernst added a commit to justinrernst/ndctl that referenced this issue Jan 2, 2024
@justinrernst
util/json: Use json_object_get_uint64() with uint64 support
e11a0ab 
If HAVE_JSON_U64=1, utils/json.c:display_hex() can call json_object_get_int64()
on a struct json_object created with json_object_new_uint64(). In the context of
'ndctl list --regions --human', this results in a static value of 0x7fffffffffffffff
being displayed for iset_id, as seen in pmem#217.

Correct hex values are observed with the use of json_object_get_uint64(). To support
builds against older json-c, use a new static inline function util_json_get_u64() to
fallback to json_object_get_int64() if HAVE_JSON_U64=0.

Link: pmem#217
Fixes: 691cd24 ("json: Add support for json_object_new_uint64()")
Signed-off-by: Justin Ernst <[email protected]>
@justinrernst justinrernst mentioned this issue Jan 2, 2024
Closed
stellarhopper pushed a commit that referenced this issue Mar 6, 2024
@justinrernst @stellarhopper
util/json: Use json_object_get_uint64() with uint64 support
731ca1f 
If HAVE_JSON_U64=1, utils/json.c:display_hex() can call
json_object_get_int64() on a struct json_object created with
json_object_new_uint64(). In the context of 'ndctl list --regions
--human', this results in a static value of 0x7fffffffffffffff being
displayed for iset_id, as seen in #217.

Correct hex values are observed with the use of
json_object_get_uint64(). To support builds against older json-c, use a
new static inline function util_json_get_u64() to fallback to
json_object_get_int64() if HAVE_JSON_U64=0.

Link: #217
Fixes: 691cd24 ("json: Add support for json_object_new_uint64()")
Signed-off-by: Justin Ernst <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vishal Verma <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@HkBkH