Re: [ndctl PATCH] test/cxl-security.sh: test dimm unlock with a large serial number

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.

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.

--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
> 
>