On Fri, Jul 03, 2026 at 02:34:36PM +0800, Richard Cheng wrote: > On Wed, Jul 01, 2026 at 05:34:03PM +0800, Alison Schofield wrote: > > The existing CXL unlock test exposed the hexadecimal-vs-decimal key > > description mismatch once cxl_test mock serial numbers were extended > > to 10 and above. Serials with bit 63 set expose a second formatting > > problem in that the kernel formats the decimal serial as signed, > > rendering it as a negative value. > > > > Extend the existing "unlock dimm" test to repeat the unlock against a > > mock memdev with a full-width serial that has bit 63 set. Refactor the > > common unlock sequence into an unlock_dimm() helper so the signedness > > case follows the same test flow as the original key lookup case. > > > > Signed-off-by: Alison Schofield <[email protected]> > > --- > > test/cxl-security | 24 ++++++++++++++++++++++++ > > test/security.sh | 16 ++++++++++++++-- > > 2 files changed, 38 insertions(+), 2 deletions(-) > > > > diff --git a/test/cxl-security b/test/cxl-security > > index 9a28ffd82b0b..39b7e001ce08 100644 > > --- a/test/cxl-security > > +++ b/test/cxl-security > > @@ -9,6 +9,30 @@ detect() > > [ -n "$id" ] || err "$LINENO" > > } > > > > +# Select the mock memdev whose serial has bit 63 set. Match on the hex > > +# spelling of 'id' because the value exceeds signed 64-bit shell > > arithmetic. > > +# A 16-digit hex value with a leading nibble of 8-f has bit 63 set. > > +detect_big_serial() > > +{ > > + local d i hex > > + > > + dev="" > > + for d in $($NDCTL list -b "$CXL_TEST_BUS" -D | jq -r '.[].dev'); do > > + i="$($NDCTL list -b "$CXL_TEST_BUS" -D -d "$d" | \ > > + jq -r '.[0].id')" > > + hex="$(printf '%x' "$i" 2>/dev/null)" || continue > > + case "${#hex}:${hex:0:1}" in > > + 16:[89a-fA-F]) > > + dev="$d" > > + id="$i" > > + break > > + ;; > > + esac > > + done > > + > > + [ -n "$dev" ] || err "$LINENO: no serial with bit 63 set found" > > +} > > + > > lock_dimm() > > { > > $NDCTL disable-dimm "$dev" > > diff --git a/test/security.sh b/test/security.sh > > index d3a840c23276..72bb570142ed 100755 > > --- a/test/security.sh > > +++ b/test/security.sh > > @@ -144,7 +144,7 @@ test_3_security_setup_and_erase() > > erase_security > > } > > > > Hi Alison, > > Just a question, > Does this cover the reboot or load-keys path? > setup_passphrase() leave the large-serial key resident in the keyring, > the this only tests lookup of an already-loaded key.
It only tests lookup of the already-loaded key. The failure was in the kernel's key lookup, not load-keys. Both setup-passphrase and load-keys install the same key description derived from the blob filename, so the kernel lookup is identical. > > The failure being addressed occurs after reboot, when the key ring is empty > and ndctl load-keys reconstructs the key from the persistent blob. > test_6_load_keys() exercise() that path, but only after detect restores the > default device. > > Could this case unlink the large-serial key, run ndctl load-keys, and then > enable and verify that the device unlocks? That would excercise the > persistence boundary relevant to the reported failure. Test 6 already covers load-keys, though not with a large serial number. Extending this test would cover that combination, but wouldn't exercise a different kernel lookup path. I'd prefer to keep this test focused on the kernel fix. The separate migration issue involves blobs saved under the old kernel's negative serial number. That requires a kernel upgrade and is documented in an ndctl issue [1] and a just submitted load-keys man page update [2]. -- Alison [1] https://github.com/pmem/ndctl/issues/299 [2] https://lore.kernel.org/nvdimm/[email protected]/ > > --Richard > > > -test_4_security_unlock() > > +unlock_dimm() > > { > > setup_passphrase > > lock_dimm > > @@ -158,6 +158,18 @@ test_4_security_unlock() > > remove_passphrase > > } > > > > +test_4_security_unlock() > > +{ > > + unlock_dimm > > + > > + if [ "$1" = "cxl" ] && check_min_kver "7.3"; then > > + detect_big_serial > > + unlock_dimm > > + # Restore the default device selection for later tests. > > + detect > > + fi > > +} > > + > > # This should always be the last nvdimm security test. > > # with security frozen, nfit_test must be removed and is no longer usable > > test_5_security_freeze() > > @@ -241,7 +253,7 @@ test_2_security_setup_and_update > > echo "Test 3, security setup and erase" > > test_3_security_setup_and_erase > > echo "Test 4, unlock dimm" > > -test_4_security_unlock > > +test_4_security_unlock "$1" > > > > # Freeze should always be the last nvdimm security test because it locks > > # security state and require nfit_test module unload. However, this does > > > > base-commit: 5fcbbee57319e718bf522436ea6595bd0f71296c > > -- > > 2.37.3 > > > >
