Re: [PATCH] s390/debug: Reset pages_per_area and nr_areas when static registration fails

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Setting `nr_areas = 0` on the error path exposes a `ZERO_SIZE_PTR` dereference in `debug_to_act_entry()` if `debug_dump()` is called with `reverse = true`.

Pre-existing issues:
- [High] debug_dump() does not check if the provided `id` is NULL before processing, leading to a kernel panic if a dynamic debug feature fails to register.
- [High] A TOCTOU race condition exists between tracing event functions (like `debug_event_common()`) and the error path of `debug_register_static()`. If `debug_register_static()` fails, this race results in a NULL pointer dereference.
--

commit e3103537d5e7bf2aad3e77c036a8ee844699d72c
Author: Mikhail Zaslonko <zaslonko@linux.ibm.com>

s390/debug: Reset pages_per_area and nr_areas when static registration fails

This patch addresses an invariant mismatch on the error path of
debug_register_static(). It sets pages_per_area and nr_areas to zero to
match the behavior where a NULL areas pointer implies zero bounds.

Closes: https://sashiko.dev/#/patchset/20260903130733.709001-1-zaslonko@linux.ibm.com?part=3

> diff --git a/arch/s390/kernel/debug.c b/arch/s390/kernel/debug.c
> index 14d2b58ad0930..0a48748bea267 100644
> --- a/arch/s390/kernel/debug.c
> +++ b/arch/s390/kernel/debug.c
> @@ -971,6 +971,8 @@ void debug_register_static(debug_info_t *id, int pages_per_area, int nr_areas)
>  		id->areas = NULL;
>  		id->active_pages = NULL;
>  		id->active_entries = NULL;

[Severity: High]
This is a pre-existing issue, but there is a race condition between tracing
event functions (like debug_event_common()) and the error path of
debug_register_static().

If debug_register_static() fails, this race results in a NULL pointer
dereference:

arch/s390/kernel/debug.c:debug_event_common() {
	if (!debug_active || !id->areas)
		return NULL;
...
	} else {
		raw_spin_lock_irqsave(&id->lock, flags);
	}
	do {
		active = get_active_entry(id);

If the lockless check in debug_event_common() passes before
debug_register_static() nullifies id->areas, debug_event_common() will wait
on the lock.

Once debug_register_static() releases the lock, debug_event_common() gets
the lock and calls get_active_entry(id) without re-checking id->areas. This
dereferences id->areas[id->active_area] (now NULL) and panics the kernel.

Can id->areas be re-checked inside the lock before calling
get_active_entry(id)?

> +		id->nr_areas = 0;
> +		id->pages_per_area = 0;

[Severity: High]
Does setting id->nr_areas to 0 expose a ZERO_SIZE_PTR dereference on
snap_id->active_pages?

If debug_dump(..., reverse = true) is later called on this static trace, it
copies the debug info by calling debug_info_alloc() with nr_areas = 0.
This allocates a ZERO_SIZE_PTR (address 0x10) for active_pages.

When debug_dump() iterates backwards, it calls debug_prev_entry(). This
checks if (p_info->act_entry == DEBUG_PROLOG_ENTRY) and immediately calls
debug_to_act_entry() before verifying if (!id->areas):

arch/s390/kernel/debug.c:debug_prev_entry() {
	if (p_info->act_entry == DEBUG_PROLOG_ENTRY)
		debug_to_act_entry(p_info);
	if (!id->areas)
		return false;

debug_to_act_entry() then reads snap_id->active_pages[snap_id->active_area],
dereferencing the unmapped ZERO_SIZE_PTR and panicking the kernel.

>  		raw_spin_unlock_irqrestore(&id->lock, flags);
>  
>  		return;

[Severity: High]
This isn't a bug introduced by this patch, but debug_dump() does not check
if the provided id pointer is NULL before processing.

If a dynamic debug feature fails to register during initialization (e.g., in
PCI), the debug ID pointer remains NULL. Subsystems may then call
debug_dump() (e.g., zpci_report_status()) with this NULL pointer on their
error reporting path.

debug_dump() passes this NULL pointer directly to debug_file_private_alloc():

arch/s390/kernel/debug.c:debug_dump() {
...
	p_info = debug_file_private_alloc(id, view);

debug_file_private_alloc() then passes it to debug_info_copy(), which
unconditionally dereferences in->name:

arch/s390/kernel/debug.c:debug_info_copy() {
...
		rc = debug_info_alloc(in->name, in->pages_per_area,
			in->nr_areas, in->buf_size, in->level, mode);

Can we add a NULL check for the id pointer at the start of debug_dump() to
prevent this panic on the error path?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910104525.3788362-1-zaslonko@linux.ibm.com?part=1