* [PATCH v3 1/3] s390/debug: Fix NULL pointer dereference in debug_set_level()
2026-09-03 13:07 [PATCH v3 0/3] s390/debug: Fix several s390dbf issues reported by AI scan Mikhail Zaslonko
@ 2026-09-03 13:07 ` Mikhail Zaslonko
2026-09-03 13:16 ` sashiko-bot
2026-09-03 13:07 ` [PATCH v3 2/3] s390/debug: Do not repeat parameter override notice on debug_set_level() Mikhail Zaslonko
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Mikhail Zaslonko @ 2026-09-03 13:07 UTC (permalink / raw)
To: linux-s390
Cc: Heiko Carstens, Alexander Gordeev, Vasily Gorbik,
Ilya Leoshkevich, Peter Oberparleiter
Commit a2cec6863709 ("s390/debug: Add s390dbf kernel parameter")
incorrectly removed a null-id check from debug_set_level(), introducing
a possible NULL pointer dereference for debug-API users that put
debug_register() results unchecked into debug_set_level().
Fix this by moving the check from the internal _debug_set_level()
variant back to the external debug_set_level() wrapper.
Fixes: a2cec6863709 ("s390/debug: Add s390dbf kernel parameter")
Signed-off-by: Mikhail Zaslonko <zaslonko@linux.ibm.com>
Reviewed-by: Peter Oberparleiter <oberpar@linux.ibm.com>
---
arch/s390/kernel/debug.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/s390/kernel/debug.c b/arch/s390/kernel/debug.c
index 14d2b58ad093..e06abf1dbc21 100644
--- a/arch/s390/kernel/debug.c
+++ b/arch/s390/kernel/debug.c
@@ -1074,9 +1074,6 @@ static void _debug_set_level(debug_info_t *id, int new_level)
{
unsigned long flags;
- if (!id)
- return;
-
if (new_level == DEBUG_OFF_LEVEL) {
pr_info("%s: switched off\n", id->name);
} else if ((new_level > DEBUG_MAX_LEVEL) || (new_level < 0)) {
@@ -1101,6 +1098,9 @@ static void _debug_set_level(debug_info_t *id, int new_level)
*/
void debug_set_level(debug_info_t *id, int new_level)
{
+ if (!id)
+ return;
+
/* Level specified via kernel parameter takes precedence */
debug_get_param(id->name, &new_level, NULL);
--
2.55.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3 2/3] s390/debug: Do not repeat parameter override notice on debug_set_level()
2026-09-03 13:07 [PATCH v3 0/3] s390/debug: Fix several s390dbf issues reported by AI scan Mikhail Zaslonko
2026-09-03 13:07 ` [PATCH v3 1/3] s390/debug: Fix NULL pointer dereference in debug_set_level() Mikhail Zaslonko
@ 2026-09-03 13:07 ` Mikhail Zaslonko
2026-09-03 13:17 ` sashiko-bot
2026-09-03 13:07 ` [PATCH v3 3/3] s390/debug: Fix race between debug area resize and event logging Mikhail Zaslonko
2026-09-03 19:33 ` [PATCH v3 0/3] s390/debug: Fix several s390dbf issues reported by AI scan Heiko Carstens
3 siblings, 1 reply; 8+ messages in thread
From: Mikhail Zaslonko @ 2026-09-03 13:07 UTC (permalink / raw)
To: linux-s390
Cc: Heiko Carstens, Alexander Gordeev, Vasily Gorbik,
Ilya Leoshkevich, Peter Oberparleiter
Commit a2cec6863709 ("s390/debug: Add s390dbf kernel parameter") calls
debug_get_param() from both debug_info_create() and debug_set_level().
Since debug_get_param() emits the override notice unconditionally, and
drivers typically call debug_set_level() right after debug_register(),
the same line is printed twice per debug area:
s390dbf: 0.0.1234: override level to 6
s390dbf: 0.0.1234: override level to 6
For areas registered per device this is multiplied by the device count.
With 's390dbf=0.0.*:6' a system with many DASDs emits a large number of
redundant lines during boot.
Add a quiet parameter to debug_get_param() and pass quiet=true from
debug_set_level(), where the override has already been announced during
registration. The remaining callers keep printing the notice.
Signed-off-by: Mikhail Zaslonko <zaslonko@linux.ibm.com>
Reviewed-by: Peter Oberparleiter <oberpar@linux.ibm.com>
---
arch/s390/kernel/debug.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/arch/s390/kernel/debug.c b/arch/s390/kernel/debug.c
index e06abf1dbc21..cf411f203571 100644
--- a/arch/s390/kernel/debug.c
+++ b/arch/s390/kernel/debug.c
@@ -182,7 +182,7 @@ static struct debug_param_t {
static int debug_param_num;
/* functions */
-static void debug_get_param(const char *name, int *level, int *pages)
+static void debug_get_param(const char *name, int *level, int *pages, bool quiet)
{
struct debug_param_t *p;
int i;
@@ -192,11 +192,13 @@ static void debug_get_param(const char *name, int *level, int *pages)
if (!glob_match(p->name, name))
continue;
if (level && p->level != PARAM_UNSET) {
- pr_info("%s: override level to %d\n", name, p->level);
+ if (!quiet)
+ pr_info("%s: override level to %d\n", name, p->level);
*level = p->level;
}
if (pages && p->pages != PARAM_UNSET) {
- pr_info("%s: override pages to %d\n", name, p->pages);
+ if (!quiet)
+ pr_info("%s: override pages to %d\n", name, p->pages);
*pages = p->pages;
}
}
@@ -251,7 +253,7 @@ static int __init s390dbf_parse(char *arg)
* regular memory allocations are possible.
*/
for (i = 0, id = __s390dbf_info; &id[i] < __s390dbf_info_end; i++)
- debug_get_param(id[i]->name, &id[i]->level, NULL);
+ debug_get_param(id[i]->name, &id[i]->level, NULL, false);
return rc;
}
@@ -395,7 +397,7 @@ static debug_info_t *debug_info_create(const char *name, int pages_per_area,
int level = DEBUG_DEFAULT_LEVEL;
debug_info_t *rc;
- debug_get_param(name, &level, &pages_per_area);
+ debug_get_param(name, &level, &pages_per_area, false);
rc = debug_info_alloc(name, pages_per_area, nr_areas, buf_size, level, ALL_AREAS);
if (!rc)
goto out;
@@ -960,7 +962,7 @@ void debug_register_static(debug_info_t *id, int pages_per_area, int nr_areas)
return;
}
- debug_get_param(id->name, &id->level, &pages_per_area);
+ debug_get_param(id->name, &id->level, &pages_per_area, false);
copy = debug_info_alloc("", pages_per_area, nr_areas, id->buf_size,
id->level, ALL_AREAS);
if (!copy) {
@@ -1101,8 +1103,11 @@ void debug_set_level(debug_info_t *id, int new_level)
if (!id)
return;
- /* Level specified via kernel parameter takes precedence */
- debug_get_param(id->name, &new_level, NULL);
+ /*
+ * Level specified via kernel parameter takes precedence. The override
+ * was already announced during registration, so stay quiet here.
+ */
+ debug_get_param(id->name, &new_level, NULL, true);
_debug_set_level(id, new_level);
}
--
2.55.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3 2/3] s390/debug: Do not repeat parameter override notice on debug_set_level()
2026-09-03 13:07 ` [PATCH v3 2/3] s390/debug: Do not repeat parameter override notice on debug_set_level() Mikhail Zaslonko
@ 2026-09-03 13:17 ` sashiko-bot
0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-09-03 13:17 UTC (permalink / raw)
To: Mikhail Zaslonko
Cc: linux-s390, Heiko Carstens, Alexander Gordeev, Vasily Gorbik,
Christian Borntraeger
> Commit a2cec6863709 ("s390/debug: Add s390dbf kernel parameter") calls
> debug_get_param() from both debug_info_create() and debug_set_level().
> Since debug_get_param() emits the override notice unconditionally, and
> drivers typically call debug_set_level() right after debug_register(),
> the same line is printed twice per debug area:
>
> s390dbf: 0.0.1234: override level to 6
> s390dbf: 0.0.1234: override level to 6
>
> For areas registered per device this is multiplied by the device count.
> With 's390dbf=0.0.*:6' a system with many DASDs emits a large number of
> redundant lines during boot.
>
> Add a quiet parameter to debug_get_param() and pass quiet=true from
> debug_set_level(), where the override has already been announced during
> registration. The remaining callers keep printing the notice.
>
> Signed-off-by: Mikhail Zaslonko <zaslonko@linux.ibm.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260903130733.709001-1-zaslonko@linux.ibm.com?part=2
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 3/3] s390/debug: Fix race between debug area resize and event logging
2026-09-03 13:07 [PATCH v3 0/3] s390/debug: Fix several s390dbf issues reported by AI scan Mikhail Zaslonko
2026-09-03 13:07 ` [PATCH v3 1/3] s390/debug: Fix NULL pointer dereference in debug_set_level() Mikhail Zaslonko
2026-09-03 13:07 ` [PATCH v3 2/3] s390/debug: Do not repeat parameter override notice on debug_set_level() Mikhail Zaslonko
@ 2026-09-03 13:07 ` Mikhail Zaslonko
2026-09-03 13:21 ` sashiko-bot
2026-09-03 19:33 ` [PATCH v3 0/3] s390/debug: Fix several s390dbf issues reported by AI scan Heiko Carstens
3 siblings, 1 reply; 8+ messages in thread
From: Mikhail Zaslonko @ 2026-09-03 13:07 UTC (permalink / raw)
To: linux-s390
Cc: Heiko Carstens, Alexander Gordeev, Vasily Gorbik,
Ilya Leoshkevich, Peter Oberparleiter
Trace functions check for non-NULL id->areas without lock to minimize
overhead. This opens a race window where a NULL pointer dereference
occurs if id->areas is set to NULL (e.g. via echo 0 > ../pages) after
the check and before id->lock is taken.
Fix this by rechecking id->areas under lock.
Signed-off-by: Mikhail Zaslonko <zaslonko@linux.ibm.com>
Reviewed-by: Peter Oberparleiter <oberpar@linux.ibm.com>
---
arch/s390/kernel/debug.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/arch/s390/kernel/debug.c b/arch/s390/kernel/debug.c
index cf411f203571..b5bf8284dbfc 100644
--- a/arch/s390/kernel/debug.c
+++ b/arch/s390/kernel/debug.c
@@ -1283,7 +1283,7 @@ void debug_set_critical(void)
debug_entry_t *debug_event_common(debug_info_t *id, int level, const void *buf,
int len)
{
- debug_entry_t *active;
+ debug_entry_t *active = NULL;
unsigned long flags;
if (!debug_active || !id->areas)
@@ -1294,6 +1294,8 @@ debug_entry_t *debug_event_common(debug_info_t *id, int level, const void *buf,
} else {
raw_spin_lock_irqsave(&id->lock, flags);
}
+ if (!id->areas)
+ goto out;
do {
active = get_active_entry(id);
memcpy(DEBUG_DATA(active), buf, min(len, id->buf_size));
@@ -1303,7 +1305,7 @@ debug_entry_t *debug_event_common(debug_info_t *id, int level, const void *buf,
len -= id->buf_size;
buf += id->buf_size;
} while (len > 0);
-
+out:
raw_spin_unlock_irqrestore(&id->lock, flags);
return active;
}
@@ -1316,7 +1318,7 @@ EXPORT_SYMBOL(debug_event_common);
debug_entry_t *debug_exception_common(debug_info_t *id, int level,
const void *buf, int len)
{
- debug_entry_t *active;
+ debug_entry_t *active = NULL;
unsigned long flags;
if (!debug_active || !id->areas)
@@ -1327,6 +1329,8 @@ debug_entry_t *debug_exception_common(debug_info_t *id, int level,
} else {
raw_spin_lock_irqsave(&id->lock, flags);
}
+ if (!id->areas)
+ goto out;
do {
active = get_active_entry(id);
memcpy(DEBUG_DATA(active), buf, min(len, id->buf_size));
@@ -1336,7 +1340,7 @@ debug_entry_t *debug_exception_common(debug_info_t *id, int level,
len -= id->buf_size;
buf += id->buf_size;
} while (len > 0);
-
+out:
raw_spin_unlock_irqrestore(&id->lock, flags);
return active;
}
@@ -1362,7 +1366,7 @@ static inline int debug_count_numargs(char *string)
debug_entry_t *__debug_sprintf_event(debug_info_t *id, int level, char *string, ...)
{
debug_sprintf_entry_t *curr_event;
- debug_entry_t *active;
+ debug_entry_t *active = NULL;
unsigned long flags;
int numargs, idx;
va_list ap;
@@ -1377,6 +1381,8 @@ debug_entry_t *__debug_sprintf_event(debug_info_t *id, int level, char *string,
} else {
raw_spin_lock_irqsave(&id->lock, flags);
}
+ if (!id->areas)
+ goto out;
active = get_active_entry(id);
curr_event = (debug_sprintf_entry_t *) DEBUG_DATA(active);
va_start(ap, string);
@@ -1385,6 +1391,7 @@ debug_entry_t *__debug_sprintf_event(debug_info_t *id, int level, char *string,
curr_event->args[idx] = va_arg(ap, long);
va_end(ap);
debug_finish_entry(id, active, level, 0);
+out:
raw_spin_unlock_irqrestore(&id->lock, flags);
return active;
@@ -1397,7 +1404,7 @@ EXPORT_SYMBOL(__debug_sprintf_event);
debug_entry_t *__debug_sprintf_exception(debug_info_t *id, int level, char *string, ...)
{
debug_sprintf_entry_t *curr_event;
- debug_entry_t *active;
+ debug_entry_t *active = NULL;
unsigned long flags;
int numargs, idx;
va_list ap;
@@ -1413,6 +1420,8 @@ debug_entry_t *__debug_sprintf_exception(debug_info_t *id, int level, char *stri
} else {
raw_spin_lock_irqsave(&id->lock, flags);
}
+ if (!id->areas)
+ goto out;
active = get_active_entry(id);
curr_event = (debug_sprintf_entry_t *)DEBUG_DATA(active);
va_start(ap, string);
@@ -1421,6 +1430,7 @@ debug_entry_t *__debug_sprintf_exception(debug_info_t *id, int level, char *stri
curr_event->args[idx] = va_arg(ap, long);
va_end(ap);
debug_finish_entry(id, active, level, 1);
+out:
raw_spin_unlock_irqrestore(&id->lock, flags);
return active;
@@ -1663,9 +1673,11 @@ static void debug_flush(debug_info_t *id, int area)
unsigned long flags;
int i, j;
- if (!id || !id->areas)
+ if (!id)
return;
raw_spin_lock_irqsave(&id->lock, flags);
+ if (!id->areas)
+ goto out;
if (area == DEBUG_FLUSH_ALL) {
id->active_area = 0;
memset(id->active_entries, 0, id->nr_areas * sizeof(int));
@@ -1680,6 +1692,7 @@ static void debug_flush(debug_info_t *id, int area)
for (i = 0; i < id->pages_per_area; i++)
memset(id->areas[area][i], 0, PAGE_SIZE);
}
+out:
raw_spin_unlock_irqrestore(&id->lock, flags);
}
--
2.55.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3 3/3] s390/debug: Fix race between debug area resize and event logging
2026-09-03 13:07 ` [PATCH v3 3/3] s390/debug: Fix race between debug area resize and event logging Mikhail Zaslonko
@ 2026-09-03 13:21 ` sashiko-bot
0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-09-03 13:21 UTC (permalink / raw)
To: Mikhail Zaslonko
Cc: Christian Borntraeger, Vasily Gorbik, Alexander Gordeev,
linux-s390, Heiko Carstens
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.
--
commit 1749ca122b22e4769c7efc7f4a52046777b678ef
Author: Mikhail Zaslonko <zaslonko@linux.ibm.com>
s390/debug: Fix race between debug area resize and event logging
Trace functions check for non-NULL id->areas without lock to minimize
overhead. This opens a race window where a NULL pointer dereference
occurs if id->areas is set to NULL after the check and before id->lock
is taken. This commit fixes the race by rechecking id->areas under lock.
> diff --git a/arch/s390/kernel/debug.c b/arch/s390/kernel/debug.c
> index cf411f203571b..b5bf8284dbfc8 100644
> --- a/arch/s390/kernel/debug.c
> +++ b/arch/s390/kernel/debug.c
[ ... ]
> @@ -1663,9 +1673,11 @@ static void debug_flush(debug_info_t *id, int area)
> unsigned long flags;
> int i, j;
>
> - if (!id || !id->areas)
> + if (!id)
> return;
> raw_spin_lock_irqsave(&id->lock, flags);
> + if (!id->areas)
> + goto out;
[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()?
> if (area == DEBUG_FLUSH_ALL) {
> id->active_area = 0;
> memset(id->active_entries, 0, id->nr_areas * sizeof(int));
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260903130733.709001-1-zaslonko@linux.ibm.com?part=3
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/3] s390/debug: Fix several s390dbf issues reported by AI scan
2026-09-03 13:07 [PATCH v3 0/3] s390/debug: Fix several s390dbf issues reported by AI scan Mikhail Zaslonko
` (2 preceding siblings ...)
2026-09-03 13:07 ` [PATCH v3 3/3] s390/debug: Fix race between debug area resize and event logging Mikhail Zaslonko
@ 2026-09-03 19:33 ` Heiko Carstens
3 siblings, 0 replies; 8+ messages in thread
From: Heiko Carstens @ 2026-09-03 19:33 UTC (permalink / raw)
To: Mikhail Zaslonko
Cc: linux-s390, Alexander Gordeev, Vasily Gorbik, Ilya Leoshkevich,
Peter Oberparleiter
On Thu, Sep 03, 2026 at 03:07:30PM +0200, Mikhail Zaslonko wrote:
> The s390dbf kernel parameter was added in commit a2cec6863709
> ("s390/debug: Add s390dbf kernel parameter"). This series fixes
> three issues found in the surrounding code.
...
> Mikhail Zaslonko (3):
> s390/debug: Fix NULL pointer dereference in debug_set_level()
> s390/debug: Do not repeat parameter override notice on
> debug_set_level()
> s390/debug: Fix race between debug area resize and event logging
>
> arch/s390/kernel/debug.c | 54 ++++++++++++++++++++++++++--------------
> 1 file changed, 36 insertions(+), 18 deletions(-)
Applied, thanks!
^ permalink raw reply [flat|nested] 8+ messages in thread