Re: [LTP] semctl01: fix SEM_STAT failures under parallel LTP runs

[linuxtestproject.agent@gmail.com]

Hi Stephen,

On Wed, 29 Jul 2026, Stephen Bertram wrote:
> semctl01: fix SEM_STAT failures under parallel LTP runs

> +	if (hidx >= sem_index)
> +		tst_res(TPASS, "IPC_INFO highest index %d >= our index %d",
> +			hidx, sem_index);
> +	else
> +		tst_res(TFAIL, "IPC_INFO highest index %d < our index %d",
> +			hidx, sem_index);

Can this branch ever take the else path? sem_index is resolved while the
set is alive, and the kernel keeps ipc_get_maxidx() at or above the index
of every live entry - ipc_rmid() only recomputes ids->max_idx when the
removed index was the maximum, and it would then find our entry.

If so the IPC_INFO case no longer asserts anything. Would it make sense
to also validate the returned struct seminfo limits (semmni, semmsl,
semopm) against /proc/sys/kernel/sem, so that IPC_INFO is really
exercised?

> +	arg.__buf = &info;
> +	max_idx = SAFE_SEMCTL(id, 0, SEM_INFO, arg);

The kernel ignores the semid argument for SEM_INFO (semctl_info() never
looks it up), so passing the set id here reads as a per-set query when it
is not.

> +	arg.buf = &dummy_ds;
> +	for (i = 0; i <= max_idx; i++) {
> +		if (semctl(i, 0, SEM_STAT, arg) == id)
> +			return i;
> +	}

The bare semctl() looks deliberate here, since unused or unreadable
indices legitimately fail with EINVAL/EACCES and SAFE_SEMCTL() would
abort. Could a short comment be added to state that, so the missing
SAFE_ wrapper is not raised again on the next read?

The braces are also not needed for the single statement body.

> +		sem_index = get_sem_idx_from_id(sem_id);
> +		if (sem_index < 0)
> +			tst_brk(TBROK, "Failed to get sem_id to idx mapping");

Including sem_id in the message would help diagnose the case where the
lookup does fail.

Verdict - Needs revision

Pre-existing issues, unrelated to this patch:

func_rmid() runs after SAFE_SEMCTL(..., IPC_RMID, ...), and that macro
assigns -1 to its first argument, which is sem_id here. So the
TST_EXP_FAIL(semop(sem_id, ...), EINVAL) check gets EINVAL from the
invalid identifier rather than from the removed set.

msgctl12.c carries the same pattern this patch fixes: index_q is taken
from IPC_INFO in setup() and then used as the MSG_STAT index.

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