[v1] tty: vt/keyboard: Hoist and reuse variable in vt_do_kdgkb_ioctl

[PATCH] tty: vt/keyboard: Hoist and reuse variable in vt_do_kdgkb_ioctl

Posted by Thorsten Blum 1 month, 1 week ago

Hoist 'len' and use it in both cases.

The last 'kbs' assignment is useless and a leftover from commit
bfb24564b5fd ("tty: vt/keyboard: use __free()"). Remove it.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 drivers/tty/vt/keyboard.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index d65fc60dd7be..6a1044f87216 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -1991,17 +1991,18 @@ static char *vt_kdskbsent(char *kbs, unsigned char cur)
 int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
 {
 	unsigned char kb_func;
+	ssize_t len;
 
 	if (get_user(kb_func, &user_kdgkb->kb_func))
 		return -EFAULT;
 
 	kb_func = array_index_nospec(kb_func, MAX_NR_FUNC);
 
+	/* size should have been a struct member */
+	len = sizeof(user_kdgkb->kb_string);
+
 	switch (cmd) {
 	case KDGKBSENT: {
-		/* size should have been a struct member */
-		ssize_t len = sizeof(user_kdgkb->kb_string);
-
 		char __free(kfree) *kbs = kmalloc(len, GFP_KERNEL);
 		if (!kbs)
 			return -ENOMEM;
@@ -2022,12 +2023,12 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
 			return -EPERM;
 
 		char __free(kfree) *kbs = strndup_user(user_kdgkb->kb_string,
-						       sizeof(user_kdgkb->kb_string));
+						       len);
 		if (IS_ERR(kbs))
 			return PTR_ERR(kbs);
 
 		guard(spinlock_irqsave)(&func_buf_lock);
-		kbs = vt_kdskbsent(kbs, kb_func);
+		vt_kdskbsent(kbs, kb_func);
 
 		return 0;
 	}
-- 
Thorsten Blum <thorsten.blum@linux.dev>
GPG: 1D60 735E 8AEF 3BE4 73B6  9D84 7336 78FD 8DFE EAD4

Re: [PATCH] tty: vt/keyboard: Hoist and reuse variable in vt_do_kdgkb_ioctl

Posted by Jiri Slaby 1 month, 1 week ago

On 26. 02. 26, 13:34, Thorsten Blum wrote:
> Hoist 'len' and use it in both cases.
> 
> The last 'kbs' assignment is useless and a leftover from commit
> bfb24564b5fd ("tty: vt/keyboard: use __free()"). Remove it.

No, kbs is set to NULL by vt_kdskbsent() when it should NOT be freed.

> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>   drivers/tty/vt/keyboard.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
> index d65fc60dd7be..6a1044f87216 100644
> --- a/drivers/tty/vt/keyboard.c
> +++ b/drivers/tty/vt/keyboard.c
...
> @@ -2022,12 +2023,12 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
>   			return -EPERM;
>   
>   		char __free(kfree) *kbs = strndup_user(user_kdgkb->kb_string,
> -						       sizeof(user_kdgkb->kb_string));
> +						       len);
>   		if (IS_ERR(kbs))
>   			return PTR_ERR(kbs);
>   
>   		guard(spinlock_irqsave)(&func_buf_lock);
> -		kbs = vt_kdskbsent(kbs, kb_func);
> +		vt_kdskbsent(kbs, kb_func);



-- 
js
suse labs

Re: [PATCH] tty: vt/keyboard: Hoist and reuse variable in vt_do_kdgkb_ioctl

Posted by Thorsten Blum 1 month, 1 week ago

On 27. Feb 2026, at 08:22, Jiri Slaby wrote:
> On 26. 02. 26, 13:34, Thorsten Blum wrote:
>> Hoist 'len' and use it in both cases.
>> The last 'kbs' assignment is useless and a leftover from commit
>> bfb24564b5fd ("tty: vt/keyboard: use __free()"). Remove it.
> 
> No, kbs is set to NULL by vt_kdskbsent() when it should NOT be freed.

Ugh thanks, I'll drop this part then.