Re: [PATCH 2/2] s390/sclp: Implement resize for sclp-vt220 console

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] tty: UAPI breakage due to asynchronous winsize initialization in open()
- [High] s390/sclp: out-of-bounds read in SCLP_VT220_SESSION_RESIZE parsing
- [Medium] s390/sclp: data race when reading shared terminal dimensions
--

commit 13e18fecbfc8440743cc8dff7c08364db9a637bf
Author: Maximilian Immanuel Brandtner <maxbr@linux.ibm.com>

s390/sclp: Implement resize for sclp-vt220 console

This patch adds support for sclp vt220 resize events to enable host-initiated
terminal resizing when using the sclp-vt220 console. This allows QEMU to
dynamically resize the console.

> diff --git a/drivers/s390/char/sclp_vt220.c b/drivers/s390/char/sclp_vt220.c
> index 7c168a66e4afe..d140617846a2d 100644
> --- a/drivers/s390/char/sclp_vt220.c
> +++ b/drivers/s390/char/sclp_vt220.c

[ ... ]

> @@ -543,6 +588,9 @@ static void sclp_vt220_receiver_fn(struct evbuf_header *evbuf)
>  	case SCLP_VT220_SESSION_DATA:
>  		/* Send input to line discipline */
>  		sclp_vt220_handle_input(buffer->data, count);
>  		tty_flip_buffer_push(&sclp_vt220_port);
>  		break;
> +	case SCLP_VT220_SESSION_RESIZE:
> +		sclp_vt220_resize_sched(buffer->data);

[Severity: High]
If the hypervisor sends an SCLP_VT220_SESSION_RESIZE event with an
evbuf->length smaller than expected, is there a risk of reading out of bounds
when the data is accessed?

This path delegates to sclp_vt220_resize_sched() without verifying if
evbuf->length is large enough to contain the 4-byte resize payload. Since the
event length is controlled by the hypervisor, could this result in an
out-of-bounds memory read when sclp_vt220_resize_sched() reads the rows and
cols fields?

> +		break;
>  	}
>  }
>  
> @@ -554,9 +602,9 @@ sclp_vt220_open(struct tty_struct *tty, struct file *filp)
>  {
>  	if (tty->count == 1) {
>  		tty_port_tty_set(&sclp_vt220_port, tty);
> -		if (!tty->winsize.ws_row && !tty->winsize.ws_col) {
> -			tty->winsize.ws_row = 24;
> -			tty->winsize.ws_col = 80;
> +		if (tty->winsize.ws_row != sclp_vt220_winsize.ws_row ||
> +		    tty->winsize.ws_col != sclp_vt220_winsize.ws_col) {

[Severity: Medium]
Is it safe to read sclp_vt220_winsize here without synchronization?

The global sclp_vt220_winsize state is updated under sclp_vt220_lock in
sclp_vt220_resize_sched(). A concurrent resize interrupt could theoretically
result in a torn read here, mixing old and new terminal dimensions.

> +			schedule_work(&sclp_vt220_resize_work);

[Severity: High]
Does this replace a safe synchronous initialization with an asynchronous one,
exposing uninitialized dimensions to userspace?

Because workqueues execute asynchronously, the open() syscall can return to
userspace before the worker thread executes tty_do_resize() to populate
tty->winsize. If an application immediately queries the terminal size with
ioctl(TIOCGWINSZ), wouldn't it read 0x0 instead of the actual dimensions?

>  		}
>  	}
>  	return 0;

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