| 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
| ||
|
Message-Id: <20251125-tty-vt-keyboard-wa-clang-scope-check-error-v1-1-f5a5ea55c578@kernel.org>
Date: Tue, 25 Nov 2025 15:54:37 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jirislaby@...nel.org>
Cc: Nick Desaulniers <nick.desaulniers+lkml@...il.com>,
Bill Wendling <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>,
linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
llvm@...ts.linux.dev, kernel test robot <lkp@...el.com>,
Nathan Chancellor <nathan@...nel.org>
Subject: [PATCH] tty: vt/keyboard: Split apart vt_do_diacrit()
After commit bfb24564b5fd ("tty: vt/keyboard: use __free()"), builds
using asm goto for put_user() and get_user() with a version of clang
older than 17 error with:
drivers/tty/vt/keyboard.c:1709:7: error: cannot jump from this asm goto statement to one of its possible targets
if (put_user(asize, &a->kb_cnt))
^
...
arch/arm64/include/asm/uaccess.h:298:2: note: expanded from macro '__put_mem_asm'
asm goto( \
^
drivers/tty/vt/keyboard.c:1687:7: note: possible target of asm goto statement
if (put_user(asize, &a->kb_cnt))
^
...
arch/arm64/include/asm/uaccess.h:342:2: note: expanded from macro '__raw_put_user'
__rpu_failed: \
^
drivers/tty/vt/keyboard.c:1697:23: note: jump exits scope of variable with __attribute__((cleanup))
void __free(kfree) *buf = kmalloc_array(MAX_DIACR, sizeof(struct kbdiacruc),
^
drivers/tty/vt/keyboard.c:1671:33: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct kbdiacr __free(kfree) *dia = kmalloc_array(MAX_DIACR, sizeof(struct kbdiacr),
^
Prior to a fix to clang's scope checker in clang 17 [1], all labels in a
function were validated as potential targets of all asm gotos in a
function, regardless of whether they actually were a target of an asm
goto call, resulting in false positive errors about skipping over
variables marked with the cleanup attribute.
To workaround this error, split up the bodies of the case statements in
vt_do_diacrit() into their own functions so that the scope checker does
not trip up on the multiple instances of __free().
Reported-by: kernel test robot <lkp@...el.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202509091702.Oc7eCRDw-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202511241835.EA8lShgH-lkp@intel.com/
Link: https://github.com/llvm/llvm-project/commit/f023f5cdb2e6c19026f04a15b5a935c041835d14 [1]
Signed-off-by: Nathan Chancellor <nathan@...nel.org>
---
drivers/tty/vt/keyboard.c | 221 ++++++++++++++++++++++++----------------------
1 file changed, 115 insertions(+), 106 deletions(-)
diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index d65fc60dd7be..3538d54d6a6a 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -1649,134 +1649,143 @@ int __init kbd_init(void)
/* Ioctl support code */
-/**
- * vt_do_diacrit - diacritical table updates
- * @cmd: ioctl request
- * @udp: pointer to user data for ioctl
- * @perm: permissions check computed by caller
- *
- * Update the diacritical tables atomically and safely. Lock them
- * against simultaneous keypresses
- */
-int vt_do_diacrit(unsigned int cmd, void __user *udp, int perm)
+static int vt_do_kdgkbdiacr(void __user *udp)
{
- int asize;
-
- switch (cmd) {
- case KDGKBDIACR:
- {
- struct kbdiacrs __user *a = udp;
- int i;
+ struct kbdiacrs __user *a = udp;
+ int i, asize;
- struct kbdiacr __free(kfree) *dia = kmalloc_array(MAX_DIACR, sizeof(struct kbdiacr),
- GFP_KERNEL);
- if (!dia)
- return -ENOMEM;
+ struct kbdiacr __free(kfree) *dia = kmalloc_array(MAX_DIACR, sizeof(struct kbdiacr),
+ GFP_KERNEL);
+ if (!dia)
+ return -ENOMEM;
- /* Lock the diacriticals table, make a copy and then
- copy it after we unlock */
- scoped_guard(spinlock_irqsave, &kbd_event_lock) {
- asize = accent_table_size;
- for (i = 0; i < asize; i++) {
- dia[i].diacr = conv_uni_to_8bit(accent_table[i].diacr);
- dia[i].base = conv_uni_to_8bit(accent_table[i].base);
- dia[i].result = conv_uni_to_8bit(accent_table[i].result);
- }
+ /* Lock the diacriticals table, make a copy and then
+ copy it after we unlock */
+ scoped_guard(spinlock_irqsave, &kbd_event_lock) {
+ asize = accent_table_size;
+ for (i = 0; i < asize; i++) {
+ dia[i].diacr = conv_uni_to_8bit(accent_table[i].diacr);
+ dia[i].base = conv_uni_to_8bit(accent_table[i].base);
+ dia[i].result = conv_uni_to_8bit(accent_table[i].result);
}
-
- if (put_user(asize, &a->kb_cnt))
- return -EFAULT;
- if (copy_to_user(a->kbdiacr, dia, asize * sizeof(struct kbdiacr)))
- return -EFAULT;
- return 0;
}
- case KDGKBDIACRUC:
- {
- struct kbdiacrsuc __user *a = udp;
- void __free(kfree) *buf = kmalloc_array(MAX_DIACR, sizeof(struct kbdiacruc),
- GFP_KERNEL);
- if (buf == NULL)
- return -ENOMEM;
+ if (put_user(asize, &a->kb_cnt))
+ return -EFAULT;
+ if (copy_to_user(a->kbdiacr, dia, asize * sizeof(struct kbdiacr)))
+ return -EFAULT;
+ return 0;
+}
- /* Lock the diacriticals table, make a copy and then
- copy it after we unlock */
- scoped_guard(spinlock_irqsave, &kbd_event_lock) {
- asize = accent_table_size;
- memcpy(buf, accent_table, asize * sizeof(struct kbdiacruc));
- }
+static int vt_do_kdgkbdiacruc(void __user *udp)
+{
+ struct kbdiacrsuc __user *a = udp;
+ int asize;
- if (put_user(asize, &a->kb_cnt))
- return -EFAULT;
- if (copy_to_user(a->kbdiacruc, buf, asize * sizeof(struct kbdiacruc)))
- return -EFAULT;
+ void __free(kfree) *buf = kmalloc_array(MAX_DIACR, sizeof(struct kbdiacruc),
+ GFP_KERNEL);
+ if (buf == NULL)
+ return -ENOMEM;
- return 0;
+ /* Lock the diacriticals table, make a copy and then
+ copy it after we unlock */
+ scoped_guard(spinlock_irqsave, &kbd_event_lock) {
+ asize = accent_table_size;
+ memcpy(buf, accent_table, asize * sizeof(struct kbdiacruc));
}
- case KDSKBDIACR:
- {
- struct kbdiacrs __user *a = udp;
- struct kbdiacr __free(kfree) *dia = NULL;
- unsigned int ct;
- int i;
+ if (put_user(asize, &a->kb_cnt))
+ return -EFAULT;
+ if (copy_to_user(a->kbdiacruc, buf, asize * sizeof(struct kbdiacruc)))
+ return -EFAULT;
- if (!perm)
- return -EPERM;
- if (get_user(ct, &a->kb_cnt))
- return -EFAULT;
- if (ct >= MAX_DIACR)
- return -EINVAL;
+ return 0;
+}
- if (ct) {
- dia = memdup_array_user(a->kbdiacr,
- ct, sizeof(struct kbdiacr));
- if (IS_ERR(dia))
- return PTR_ERR(dia);
- }
+static int vt_do_kdskbdiacr(void __user *udp, int perm)
+{
+ struct kbdiacrs __user *a = udp;
+ struct kbdiacr __free(kfree) *dia = NULL;
+ unsigned int ct;
+ int i;
- guard(spinlock_irqsave)(&kbd_event_lock);
- accent_table_size = ct;
- for (i = 0; i < ct; i++) {
- accent_table[i].diacr =
- conv_8bit_to_uni(dia[i].diacr);
- accent_table[i].base =
- conv_8bit_to_uni(dia[i].base);
- accent_table[i].result =
- conv_8bit_to_uni(dia[i].result);
- }
+ if (!perm)
+ return -EPERM;
+ if (get_user(ct, &a->kb_cnt))
+ return -EFAULT;
+ if (ct >= MAX_DIACR)
+ return -EINVAL;
- return 0;
+ if (ct) {
+ dia = memdup_array_user(a->kbdiacr,
+ ct, sizeof(struct kbdiacr));
+ if (IS_ERR(dia))
+ return PTR_ERR(dia);
}
- case KDSKBDIACRUC:
- {
- struct kbdiacrsuc __user *a = udp;
- unsigned int ct;
- void __free(kfree) *buf = NULL;
+ guard(spinlock_irqsave)(&kbd_event_lock);
+ accent_table_size = ct;
+ for (i = 0; i < ct; i++) {
+ accent_table[i].diacr =
+ conv_8bit_to_uni(dia[i].diacr);
+ accent_table[i].base =
+ conv_8bit_to_uni(dia[i].base);
+ accent_table[i].result =
+ conv_8bit_to_uni(dia[i].result);
+ }
- if (!perm)
- return -EPERM;
+ return 0;
+}
- if (get_user(ct, &a->kb_cnt))
- return -EFAULT;
+static int vt_do_kdskbdiacruc(void __user *udp, int perm)
+{
+ struct kbdiacrsuc __user *a = udp;
+ unsigned int ct;
+ void __free(kfree) *buf = NULL;
- if (ct >= MAX_DIACR)
- return -EINVAL;
+ if (!perm)
+ return -EPERM;
- if (ct) {
- buf = memdup_array_user(a->kbdiacruc,
- ct, sizeof(struct kbdiacruc));
- if (IS_ERR(buf))
- return PTR_ERR(buf);
- }
- guard(spinlock_irqsave)(&kbd_event_lock);
- if (ct)
- memcpy(accent_table, buf,
- ct * sizeof(struct kbdiacruc));
- accent_table_size = ct;
- return 0;
+ if (get_user(ct, &a->kb_cnt))
+ return -EFAULT;
+
+ if (ct >= MAX_DIACR)
+ return -EINVAL;
+
+ if (ct) {
+ buf = memdup_array_user(a->kbdiacruc,
+ ct, sizeof(struct kbdiacruc));
+ if (IS_ERR(buf))
+ return PTR_ERR(buf);
}
+ guard(spinlock_irqsave)(&kbd_event_lock);
+ if (ct)
+ memcpy(accent_table, buf,
+ ct * sizeof(struct kbdiacruc));
+ accent_table_size = ct;
+ return 0;
+}
+
+/**
+ * vt_do_diacrit - diacritical table updates
+ * @cmd: ioctl request
+ * @udp: pointer to user data for ioctl
+ * @perm: permissions check computed by caller
+ *
+ * Update the diacritical tables atomically and safely. Lock them
+ * against simultaneous keypresses
+ */
+int vt_do_diacrit(unsigned int cmd, void __user *udp, int perm)
+{
+ switch (cmd) {
+ case KDGKBDIACR:
+ return vt_do_kdgkbdiacr(udp);
+ case KDGKBDIACRUC:
+ return vt_do_kdgkbdiacruc(udp);
+ case KDSKBDIACR:
+ return vt_do_kdskbdiacr(udp, perm);
+ case KDSKBDIACRUC:
+ return vt_do_kdskbdiacruc(udp, perm);
}
return 0;
}
---
base-commit: da218406dd50e0ac96bb383de4edd208286efe70
change-id: 20251125-tty-vt-keyboard-wa-clang-scope-check-error-be4d5d7d5f2e
Best regards,
--
Nathan Chancellor <nathan@...nel.org>
Powered by blists - more mailing lists