On 08. 06. 22, 10:02, David Laight wrote:
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@xxxxxxx>
---
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.
Agreed. The code is far from perfect. We might try to return ENOSPC and
watch what breaks. (And decouple the double "?:" operator as it makes
things only worse.)
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.
Note that the patches try NOT to change the behavior in any way. If they
do, it's likely a bug. They are first front cleanup. Definitely more to
come. Either from me, or others -- patches welcome ;).
thanks,
--
js
suse labs
