[PATCH 0/2] s390/char: Enable CONTEXT_ANALYSIS

[PATCH 0/2] s390/char: Enable CONTEXT_ANALYSIS

[Heiko Carstens]

Enable CONTEXT_ANALYSYS for s390/char drivers.

Static code checking for acquiring and releasing locks used to be done
with sparse. That was removed with [1] and replaced with a clang based
approach [2]. The new approach requires that each subsystem needs to be
explicitly enabled for checking.

Do that for drivers/s390/char. Add a __must_hold() attribute to
raw3215_make_room() to address the only valid warning.
Then enable CONTEXT_ANALYSIS.

Thanks,
Heiko

[1] 5b63d0ae94cc ("compiler-context-analysis: Remove Sparse support")
[2] 3269701cb256 ("compiler-context-analysis: Add infrastructure for Context Analysis with Clang")

Heiko Carstens (2):
  s390/con3215: Add __must_hold() attribute to raw3215_make_room()
  s390/char: Enable CONTEXT_ANALYSIS

 drivers/s390/char/Makefile  | 2 ++
 drivers/s390/char/con3215.c | 1 +
 2 files changed, 3 insertions(+)

-- 
2.53.0

[PATCH 1/2] s390/con3215: Add __must_hold() attribute to raw3215_make_room()

[Heiko Carstens]

Add __must_hold() attribute to raw3215_make_room() in order to let
clang's context analysis know that "get_ccwdev_lock(raw->cdev)" must be
held on function entry. This can also be easily verified when inspecting
the function.

Without this annotation this leads to a valid warning when context
analysis is enabled:

drivers/s390/char/con3215.c:485:9: warning:
  expecting spinlock 'raw->cdev->ccwlock' to be held at start of each loop [-Wthread-safety-analysis]
  485 |         while (RAW3215_BUFFER_SIZE - raw->count < length) {

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 drivers/s390/char/con3215.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/s390/char/con3215.c b/drivers/s390/char/con3215.c
index 46265f4fbfd8..57fefa8986d6 100644
--- a/drivers/s390/char/con3215.c
+++ b/drivers/s390/char/con3215.c
@@ -481,6 +481,7 @@ static unsigned int raw3215_drop(struct raw3215_info *raw)
  */
 static unsigned int raw3215_make_room(struct raw3215_info *raw,
 				      unsigned int length, bool drop)
+	__must_hold(get_ccwdev_lock(raw->cdev))
 {
 	while (RAW3215_BUFFER_SIZE - raw->count < length) {
 		if (drop)
-- 
2.53.0

Re: [PATCH 1/2] s390/con3215: Add __must_hold() attribute to raw3215_make_room()

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Use-After-Free during device removal due to concurrent lock dropping and missing console unregistration.
- [High] Sleeping in atomic context during panic/reboot on PREEMPT_RT.
--

--- Patch [1]: [PATCH 1/2] s390/con3215: Add __must_hold() attribute to raw3215_make_room() ---
commit bdab109aec8f8a62fc73654df5f2f42b168ab432
Author: Heiko Carstens <hca@linux.ibm.com>

s390/con3215: Add __must_hold() attribute to raw3215_make_room()
    
Add __must_hold() attribute to raw3215_make_room() in order to let
clang's context analysis know that "get_ccwdev_lock(raw->cdev)" must be
held on function entry.

> diff --git a/drivers/s390/char/con3215.c b/drivers/s390/char/con3215.c
> index 46265f4fbfd80..57fefa8986d6b 100644
> --- a/drivers/s390/char/con3215.c
> +++ b/drivers/s390/char/con3215.c
> @@ -481,6 +481,7 @@ static unsigned int raw3215_drop(struct raw3215_info *raw)
>   */
>  static unsigned int raw3215_make_room(struct raw3215_info *raw,
>  				      unsigned int length, bool drop)
> +	__must_hold(get_ccwdev_lock(raw->cdev))
>  {
>  	while (RAW3215_BUFFER_SIZE - raw->count < length) {
>  		if (drop)

[Severity: Critical]
This is a pre-existing issue, but can this sequence lead to a use-after-free
if a concurrent device unbind occurs?

When raw3215_make_room() yields the lock and sleeps, it does so without any
reference counting:

drivers/s390/char/con3215.c:raw3215_make_room() {
    ...
		/* there might be another cpu waiting for the lock */
		spin_unlock(get_ccwdev_lock(raw->cdev));
		udelay(100);
		spin_lock(get_ccwdev_lock(raw->cdev));
    ...
}

If con3215_write() calls raw3215_make_room() and drops the lock here,
could a concurrent sysfs unbind invoke raw3215_remove(), which nullifies
raw3215[0] and calls kfree(raw) without unregistering the console or
synchronizing with the ccwlock?

When raw3215_make_room() wakes up from the udelay, it appears it would
directly dereference the freed raw pointer when calling spin_lock().

[Severity: High]
This is also a pre-existing issue, but does this spin_lock() introduce a
sleepable lock acquisition into an atomic panic context on PREEMPT_RT?

When the system panics or reboots, con3215_notify() uses
spin_trylock_irqsave() to avoid sleeping, as local IRQs are disabled.

If it then calls raw3215_make_room() and the buffer is full, the code drops
the lock and unconditionally calls spin_lock(). On PREEMPT_RT kernels,
spin_lock() maps to a sleepable rt_mutex.

Could calling this with hardware interrupts disabled violate wait-context
rules and trigger a sleeping-in-atomic bug?

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

Re: [PATCH 1/2] s390/con3215: Add __must_hold() attribute to raw3215_make_room()

[Sven Schnelle]

Heiko Carstens <hca@linux.ibm.com> writes:

> Add __must_hold() attribute to raw3215_make_room() in order to let
> clang's context analysis know that "get_ccwdev_lock(raw->cdev)" must be
> held on function entry. This can also be easily verified when inspecting
> the function.
>
> Without this annotation this leads to a valid warning when context
> analysis is enabled:
>
> drivers/s390/char/con3215.c:485:9: warning:
>   expecting spinlock 'raw->cdev->ccwlock' to be held at start of each loop [-Wthread-safety-analysis]
>   485 |         while (RAW3215_BUFFER_SIZE - raw->count < length) {
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  drivers/s390/char/con3215.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/s390/char/con3215.c b/drivers/s390/char/con3215.c
> index 46265f4fbfd8..57fefa8986d6 100644
> --- a/drivers/s390/char/con3215.c
> +++ b/drivers/s390/char/con3215.c
> @@ -481,6 +481,7 @@ static unsigned int raw3215_drop(struct raw3215_info *raw)
>   */
>  static unsigned int raw3215_make_room(struct raw3215_info *raw,
>  				      unsigned int length, bool drop)
> +	__must_hold(get_ccwdev_lock(raw->cdev))
>  {
>  	while (RAW3215_BUFFER_SIZE - raw->count < length) {
>  		if (drop)

Reviewed-by: Sven Schnelle <svens@linux.ibm.com>

[PATCH 2/2] s390/char: Enable CONTEXT_ANALYSIS

[Heiko Carstens]

All character drivers pass clang's compile time context analysis.
Therefore enable CONTEXT_ANALYSIS.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 drivers/s390/char/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/s390/char/Makefile b/drivers/s390/char/Makefile
index 126a87c3c6f8..0ad960d000a2 100644
--- a/drivers/s390/char/Makefile
+++ b/drivers/s390/char/Makefile
@@ -3,6 +3,8 @@
 # S/390 character devices
 #
 
+CONTEXT_ANALYSIS := y
+
 ifdef CONFIG_FUNCTION_TRACER
 # Do not trace early setup code
 CFLAGS_REMOVE_sclp_early_core.o	= $(CC_FLAGS_FTRACE)
-- 
2.53.0

Re: [PATCH 2/2] s390/char: Enable CONTEXT_ANALYSIS

[sashiko-bot]

> All character drivers pass clang's compile time context analysis.
> Therefore enable CONTEXT_ANALYSIS.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

Re: [PATCH 2/2] s390/char: Enable CONTEXT_ANALYSIS

[Sven Schnelle]

Heiko Carstens <hca@linux.ibm.com> writes:

> All character drivers pass clang's compile time context analysis.
> Therefore enable CONTEXT_ANALYSIS.
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  drivers/s390/char/Makefile | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/s390/char/Makefile b/drivers/s390/char/Makefile
> index 126a87c3c6f8..0ad960d000a2 100644
> --- a/drivers/s390/char/Makefile
> +++ b/drivers/s390/char/Makefile
> @@ -3,6 +3,8 @@
>  # S/390 character devices
>  #
>  
> +CONTEXT_ANALYSIS := y
> +
>  ifdef CONFIG_FUNCTION_TRACER
>  # Do not trace early setup code
>  CFLAGS_REMOVE_sclp_early_core.o	= $(CC_FLAGS_FTRACE)

Reviewed-by: Sven Schnelle <svens@linux.ibm.com>

Re: [PATCH 0/2] s390/char: Enable CONTEXT_ANALYSIS

[Heiko Carstens]

On Thu, Aug 06, 2026 at 05:53:01PM +0200, Heiko Carstens wrote:
> Enable CONTEXT_ANALYSYS for s390/char drivers.
...
> Heiko Carstens (2):
>   s390/con3215: Add __must_hold() attribute to raw3215_make_room()
>   s390/char: Enable CONTEXT_ANALYSIS
> 
>  drivers/s390/char/Makefile  | 2 ++
>  drivers/s390/char/con3215.c | 1 +
>  2 files changed, 3 insertions(+)

Applied.