Re: [LTP] lapi/keyctl.h: Add fallback definitions for extended ops

[linuxtestproject.agent@gmail.com]

Hi Andrea,

On Wed, 16 Sep 2026 16:03:27 +0200, Andrea Cervesato <andrea.cervesato@suse.com> wrote:
> lapi/keyctl.h: Add fallback definitions for extended ops

--- [PATCH 1/36] ---

> +#ifndef HAVE_STRUCT_KEYCTL_DH_PARAMS
> +struct keyctl_dh_params {
> +	int32_t priv;
> +	int32_t prime;
> +	int32_t base;
> +};
> +#endif

HAVE_STRUCT_KEYCTL_DH_PARAMS_PRIV is probed in m4/ltp-keyutils.m4 for Leap
42.2 compatibility but is never handled here. When
HAVE_STRUCT_KEYCTL_DH_PARAMS is defined without
HAVE_STRUCT_KEYCTL_DH_PARAMS_PRIV, accessing dh_params->priv causes a build
failure. Add an alias:

#if defined(HAVE_STRUCT_KEYCTL_DH_PARAMS) && \
	!defined(HAVE_STRUCT_KEYCTL_DH_PARAMS_PRIV)
# define priv private
#endif

--- [PATCH 2/36] ---

> +	if (rval == -1) {
> +		tst_brk_(file, lineno, TBROK | TERRNO,
> +			 "add_key(%s, '%s', %p, %ld, %d) failed",
> +			 type, desc, payload, size, keyring);
> +	} else if (rval < -1) {
> +		tst_brk_(file, lineno, TBROK | TERRNO,
> +			 "Invalid add_key(%s, '%s', %p, %ld, %d) return value %d",
> +			 type, desc, payload, size, keyring, rval);
> +	}

The size parameter has type size_t, but %ld is used in the format strings.
Use %zu instead.

> +#define SAFE_ADD_KEY(type, desc, payload, size, keyring) \
> +	safe_add_key(__FILE__, __LINE__, \
> +                            (type), (desc), (payload), (size), (keyring))

Use tabs instead of spaces for indentation.

--- [PATCH 4/36] ---

> +#define SAFE_NEW_RING(desc) \
> +	safe_add_key(__FILE__, __LINE__, "keyring", (desc), NULL, 0, KEY_SPEC_PROCESS_KEYRING)
> +
> +#define SAFE_NEW_USER_KEY(desc, payload, plen, ring) \
> +	safe_add_key(__FILE__, __LINE__, "user", (desc), (payload), (plen), (ring))

The SAFE_* prefix is reserved for LTP core library headers in include/.
Rename these macros without the SAFE_ prefix (e.g. NEW_RING and
NEW_USER_KEY) or move them to include/lapi/keyctl.h.

--- [PATCH 9/36] ---

> +	rc = SAFE_KEYCTL(KEYCTL_GET_SECURITY, key, (unsigned long)buf, sizeof(buf), 0);
> +
> +	if (buf[0] != '\0')
> +		tst_res(TFAIL, "empty label is not NUL terminated");

Checking buf[0] != '\0' unconditionally fails on systems where an LSM
(such as SELinux or Smack) is active and returns a non-empty security label.
Check buf[rc - 1] != '\0' instead to verify NUL-termination, or only check
buf[0] == '\0' when rc == 1.

--- [PATCH 14/36] ---

> +static void run(void)
> +{
> +	SAFE_KEYCTL(KEYCTL_LINK, key_a, ring_a, 0, 0);
> +	TST_EXP_PASS(keyctl(KEYCTL_MOVE, key_a, ring_a, ring_b, 0));
> +
> +	TST_EXP_EQ_LI(keyctl(KEYCTL_SEARCH, ring_b, "user", KEY_DESC), key_a);
> +}

When running multiple iterations (-i), key_excl is displaced from ring_b
in the first iteration and is never re-linked to ring_b. Subsequent
iterations therefore do not test displacement. Re-link key_excl to ring_b
in run() before moving key_a.

--- [PATCH 17/36] ---

> 		case 'i':
> -			iterations = SAFE_STRTOL(optarg, 0, INT_MAX);
> +			tst_test->iterations = SAFE_STRTOL(optarg, 0, UINT_MAX);

Passing UINT_MAX to SAFE_STRTOL() breaks on 32-bit architectures because
safe_strtol() takes signed long, where UINT_MAX (0xFFFFFFFF) converts to
-1L. This makes safe_strtol() reject any positive iteration count as out of
range 0 to -1. Use SAFE_STRTOUL(optarg, 0, UINT_MAX) or keep INT_MAX.

--- [PATCH 35/36] ---

> +	.ulimit = (const struct tst_ulimit_val []) {
> +		{RLIMIT_NOFILE, 524288},
> +		{}
> +	},

Setting .ulimit requires .needs_root = 1 when raising limits above the
current maximum. Without root privileges, safe_setrlimit() fails with
EPERM and aborts the test with TBROK. Either drop .ulimit or add
.needs_root = 1.

Verdict - Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp