Re: [PATCH v3 3/3] s390/debug: Fix race between debug area resize and event logging

[Mikhail Zaslonko <zaslonko@linux.ibm.com>]

On 03-Sep-26 15:21, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Preexisting NULL pointer dereference in debug_info_copy() if debug_register_static() fails to allocate memory.
> --
> 

> 
> [Severity: High]
> This isn't a bug introduced by this patch, but I noticed a potential NULL
> pointer dereference in debug_info_copy() if debug_register_static() fails
> to allocate memory.
> 
> In arch/s390/kernel/debug.c:debug_register_static(), if the allocation fails:
> 
> 	if (!copy) {
> 		pr_err("Registering debug feature %s failed\n", id->name);
> 
> 		/* Clear pointers to prevent tracing into released initdata. */
> 		raw_spin_lock_irqsave(&id->lock, flags);
> 		id->areas = NULL;
> 		id->active_pages = NULL;
> 		id->active_entries = NULL;
> 		raw_spin_unlock_irqrestore(&id->lock, flags);
> 
> 		return;
> 	}
> 
> This sets id->areas to NULL but leaves id->pages_per_area and id->nr_areas
> with their original EARLY_PAGES and EARLY_AREAS values.
> 
> Later, if debug_info_copy() is called when a user accesses the exposed
> debugfs file:
> 
> 	for (i = 0; i < in->nr_areas; i++) {
> 		for (j = 0; j < in->pages_per_area; j++)
> 			memcpy(rc->areas[i][j], in->areas[i][j], PAGE_SIZE);
> 
> Will this cause a NULL pointer dereference when accessing in->areas[i][j]
> since in->areas is NULL but the loop still executes based on in->nr_areas?
> Should we reset id->pages_per_area and id->nr_areas to 0 on the error path
> in debug_register_static(), or add a NULL check in debug_info_copy()?
> 

The scenario above is unrealistic since upon debug_register_static() failure,
the _debug_register(id) is not called. So no debugfs entry exists for this id
on the failure path and a user can't access it and initiate debug_info_copy().
Thus, severity is arguably High.

Nevertheless, it's a fair finding. Everywhere in the debug feature code a NULL
areas pointer implies zero bounds: debug_info_alloc() sets areas to NULL when
pages_per_area is 0, and debug_areas_swap() swaps areas, nr_areas and
pages_per_area together. The error path of debug_register_static() is the only
exception. That's worth fixing even though nothing can currently observe it.

I will send a separate small patch.