linux-kernel - RE: [PATCH 16/36] tty/vt: consolemap: check put_user() in con_get

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]

Date:   Wed, 8 Jun 2022 08:02:30 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Jiri Slaby' <jslaby@...e.cz>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
CC:     "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 16/36] tty/vt: consolemap: check put_user() in
 con_get_unimap()

From: Jiri Slaby
> Sent: 07 June 2022 11:49
> 
> Only the return value of copy_to_user() is checked in con_get_unimap().
> Do the same for put_user() of the count too.
> 
> Signed-off-by: Jiri Slaby <jslaby@...e.cz>
> ---
>  drivers/tty/vt/consolemap.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
> index 831450f2bfd1..92b5dddb00d9 100644
> --- a/drivers/tty/vt/consolemap.c
> +++ b/drivers/tty/vt/consolemap.c
> @@ -813,7 +813,8 @@ int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct,
>  	console_unlock();
>  	if (copy_to_user(list, unilist, min(ect, ct) * sizeof(*unilist)))
>  		ret = -EFAULT;
> -	put_user(ect, uct);
> +	if (put_user(ect, uct))
> +		ret = -EFAULT;
>  	kvfree(unilist);
>  	return ret ? ret : (ect <= ct) ? 0 : -ENOMEM;
>  }

How is the user expected to check the result of this code?

AFAICT -ENOMEM is returned if either kmalloc() fails or
the user buffer is too short?
Looks pretty hard to detect which.

I've not looked at the effect of all the patches, but setting
'ret = -ENOMEM' and breaking the loop when the array is too
small would simplify things.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)