linux-kernel - Re: [PATCH v2] vc_screen: modify vcs_size() handling in vcs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <face2b1b-0f2e-4a79-a71b-79681fc56273@t-8ch.de>
Date:   Tue, 28 Feb 2023 01:45:56 +0000
From:   linux@...ssschuh.net
To:     George Kennedy <george.kennedy@...cle.com>
Cc:     jirislaby@...nel.org, gregkh@...uxfoundation.org,
        torvalds@...ux-foundation.org, sfr@...b.auug.org.au,
        akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
        linux-serial@...r.kernel.org, regressions@...ts.linux.dev
Subject: Re: [PATCH v2] vc_screen: modify vcs_size() handling in vcs_read()

Hi,

On Mon, Feb 27, 2023 at 03:21:41PM -0500, George Kennedy wrote:
> Restore the vcs_size() handling in vcs_read() to what
> it had been in previous version.

This still seems to be broken for me.

Testcase:

# cat /dev/vcsa1
[.. data, looks complete ..]
cat: /dev/vcsa1: No such device or address

"ret" is still unconditionally overwritten with -ENXIO at the beginning
of the loop.
And when we break the loop because everything has been read in
`if (pos >= size)` then this error is returned to userspace instead of
just `0`.

I still need the patch from 

https://lore.kernel.org/lkml/20230220064612.1783-1-linux@weissschuh.net/

> Fixes: 226fae124b2d ("vc_screen: move load of struct vc_data pointer in vcs_read() to avoid UAF")
> Suggested-by: Jiri Slaby <jirislaby@...nel.org>
> Signed-off-by: George Kennedy <george.kennedy@...cle.com>
> ---
> v2: added Fixes
> 
>  drivers/tty/vt/vc_screen.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
> index f566eb1839dc..c0a2273bb998 100644
> --- a/drivers/tty/vt/vc_screen.c
> +++ b/drivers/tty/vt/vc_screen.c
> @@ -414,10 +414,8 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
>  		 */
>  		size = vcs_size(vc, attr, uni_mode);
>  		if (size < 0) {
> -			if (read)
> -				break;
>  			ret = size;
> -			goto unlock_out;
> +			break;
>  		}
>  		if (pos >= size)
>  			break;
> -- 
> 2.31.1
>