| Message ID | 20220722061133.7765-1-yixingrui@linux.alibaba.com |
|---|---|
| State | New |
| Headers | show |
| Series | vt: remove old FONT ioctls definitions in uapi | expand |
On Fri, Jul 22, 2022 at 02:11:33PM +0800, Xingrui Yi wrote: > As was demonstrated by commit ff2047fb755d ("vt: drop old FONT ioctls"), > old font ioctls like PIO_FONT have been deleted and KDFONTOP ioctl is used > for years instead. > > However, unused definitions of these ioctl numbers and "strut > consolefontdesc" still exist in a uapi header. They could have been removed > since no userspace was using them. Otherwise they will become a > misleading for users, and users will fail with ENOTTY with wrong call. > > We are moving old font ioctl numbers definitions here to guide users to > use KDFONTOP. > > Signed-off-by: Xingrui Yi <yixingrui@linux.alibaba.com> > --- > include/uapi/linux/kd.h | 13 ------------- > 1 file changed, 13 deletions(-) > > diff --git a/include/uapi/linux/kd.h b/include/uapi/linux/kd.h > index ee929ece4112..d739b17fc942 100644 > --- a/include/uapi/linux/kd.h > +++ b/include/uapi/linux/kd.h > @@ -6,19 +6,6 @@ > > /* 0x4B is 'K', to avoid collision with termios and vt */ > > -#define GIO_FONT 0x4B60 /* gets font in expanded form */ > -#define PIO_FONT 0x4B61 /* use font in expanded form */ > - > -#define GIO_FONTX 0x4B6B /* get font using struct consolefontdesc */ > -#define PIO_FONTX 0x4B6C /* set font using struct consolefontdesc */ > -struct consolefontdesc { > - unsigned short charcount; /* characters in font (256 or 512) */ > - unsigned short charheight; /* scan lines per character (1-32) */ > - char __user *chardata; /* font data in expanded form */ > -}; > - > -#define PIO_FONTRESET 0x4B6D /* reset to default font */ > - > #define GIO_CMAP 0x4B70 /* gets colour palette on VGA+ */ > #define PIO_CMAP 0x4B71 /* sets colour palette on VGA+ */ > Did you rebuild a distribution like Debian to verify that these values and structure are not used anywhere? thanks, greg k-h
On Thu, Jul 28, 2022 at 10:26:10AM +0200, Greg KH wrote: > On Fri, Jul 22, 2022 at 02:11:33PM +0800, Xingrui Yi wrote: > > As was demonstrated by commit ff2047fb755d ("vt: drop old FONT ioctls"), > > old font ioctls like PIO_FONT have been deleted and KDFONTOP ioctl is used > > for years instead. > > > > However, unused definitions of these ioctl numbers and "strut > > consolefontdesc" still exist in a uapi header. They could have been removed > > since no userspace was using them. Otherwise they will become a > > misleading for users, and users will fail with ENOTTY with wrong call. > > -#define GIO_FONT 0x4B60 /* gets font in expanded form */ > > -#define PIO_FONT 0x4B61 /* use font in expanded form */ > > -#define GIO_FONTX 0x4B6B /* get font using struct consolefontdesc */ > > -#define PIO_FONTX 0x4B6C /* set font using struct consolefontdesc */ > > -struct consolefontdesc { > > -#define PIO_FONTRESET 0x4B6D /* reset to default font */ > Did you rebuild a distribution like Debian to verify that these values > and structure are not used anywhere? I just did. https://codesearch.debian.net lists packages to check: aalib busybox cde console-cyrillic dietlibc doclifter emscripten fpc gargoyle-free gcc-10 gcc-11 gcc-12 gcc-9 gcc-arm-none-eabi gcc-avr gcc-riscv64-unknown-elf gcc-snapshot hurd kbd kfreebsd-10 lazarus libabigail libexplain linux llvm-toolchain-11 llvm-toolchain-12 llvm-toolchain-13 llvm-toolchain-14 llvm-toolchain-9 llvm-toolchain-snapshot lynx manpages-l10n sequeler strace stress-ng systemd trinity vala valgrind Of these, I've ignored the kernels, and built only the latest released version of a compiler. .--====[ aalib ] aalinux.c: In function ‘linux_init’: aalinux.c:106:34: error: storage size of ‘desc’ isn’t known 106 | struct consolefontdesc desc; | ^~~~ aalinux.c:109:26: error: ‘GIO_FONTX’ undeclared (first use in this function) 109 | i = ioctl (fd, GIO_FONTX, &desc); | ^~~~~~~~~ `---- .--====[ kbd ] kdfontop.c: In function ‘kfont_restore_font’: kdfontop.c:27:23: error: ‘PIO_FONTRESET’ undeclared (first use in this function) 27 | if (ioctl(fd, PIO_FONTRESET, 0)) { | ^~~~~~~~~~~~~ kdfontop.c: In function ‘get_font_giofontx’: kdfontop.c:89:32: error: storage size of ‘cfd’ isn’t known kdfontop.c:102:30: error: ‘GIO_FONTX’ undeclared kdfontop.c: In function ‘get_font_giofont’: kdfontop.c:137:30: error: ‘GIO_FONT’ undeclared kdfontop.c: In function ‘put_font_piofontx’: kdfontop.c:242:32: error: storage size of ‘cfd’ isn’t known kdfontop.c:262:31: error: ‘PIO_FONTX’ undeclared kdfontop.c: In function ‘put_font_piofont’: kdfontop.c:299:31: error: ‘PIO_FONT’ undeclared `---- .--====[ gcc-12 ] ../../../../src/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cpp:850:29: error: 'GIO_FONT' was not declared in this scope 850 | unsigned IOCTL_GIO_FONT = GIO_FONT; | ^~~~~~~~ `---- .--====[ llvm-toolchain-14 ] /<<PKGBUILDDIR>>/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp:865:29: error: use of undeclared identifier 'GIO_FONT' unsigned IOCTL_GIO_FONT = GIO_FONT; ^ /<<PKGBUILDDIR>>/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp:898:29: error: use of undeclared identifier 'PIO_FONT' unsigned IOCTL_PIO_FONT = PIO_FONT; ^ `---- .--====[ libexplain ] libexplain/buffer/consolefontdesc.c: In function ‘explain_buffer_consolefontdesc’: libexplain/buffer/consolefontdesc.c:34:48: error: invalid application of ‘sizeof’ to incomplete type ‘c onst struct consolefontdesc’ 34 | if (explain_is_efault_pointer(value, sizeof(*value))) | ^ libexplain/buffer/consolefontdesc.c:43:14: error: invalid use of undefined type ‘const struct consolefo ntdesc’ 43 | value->charcount, | ^~ `---- .--====[ trinity ] In file included from ioctls/vt.c:8: ioctls/vt.c:48:15: error: ‘GIO_FONT’ undeclared here (not in a function) 48 | IOCTL(GIO_FONT), | ^~~~~~~~ include/ioctls.h:53:22: note: in definition of macro ‘IOCTL’ 53 | { .request = _request, .name = #_request, } | ^~~~~~~~ ioctls/vt.c:49:15: error: ‘PIO_FONT’ undeclared here (not in a function) 49 | IOCTL(PIO_FONT), | ^~~~~~~~ ioctls/vt.c:50:15: error: ‘GIO_FONTX’ undeclared here (not in a function) ioctls/vt.c:51:15: error: ‘PIO_FONTX’ undeclared here (not in a function) ioctls/vt.c:52:15: error: ‘PIO_FONTRESET’ undeclared here (not in a function) `---- So there _are_ failures in userspace. But let's take a look at them: * aalib: dead upstream since 2001, this code is broken * kbd: uses KDFONTOP, falls back if unsupported * libsanitizer (bundled in gcc, llvm-toolchain): can drop * libexplain: can drop * trinity: can drop So we have one real failure, one fallback, and a few "lists all ioctls". I'd say that's acceptable. Especially because presence of code that list all ioctls means we would never done any clean-up otherwise. Meow!
On Fri, Jul 29, 2022 at 11:21:41PM +0200, Adam Borowski wrote: > On Thu, Jul 28, 2022 at 10:26:10AM +0200, Greg KH wrote: > > On Fri, Jul 22, 2022 at 02:11:33PM +0800, Xingrui Yi wrote: > > > As was demonstrated by commit ff2047fb755d ("vt: drop old FONT ioctls"), > > > old font ioctls like PIO_FONT have been deleted and KDFONTOP ioctl is used > > > for years instead. > > > > > > However, unused definitions of these ioctl numbers and "strut > > > consolefontdesc" still exist in a uapi header. They could have been removed > > > since no userspace was using them. Otherwise they will become a > > > misleading for users, and users will fail with ENOTTY with wrong call. > > > > -#define GIO_FONT 0x4B60 /* gets font in expanded form */ > > > -#define PIO_FONT 0x4B61 /* use font in expanded form */ > > > -#define GIO_FONTX 0x4B6B /* get font using struct consolefontdesc */ > > > -#define PIO_FONTX 0x4B6C /* set font using struct consolefontdesc */ > > > -struct consolefontdesc { > > > -#define PIO_FONTRESET 0x4B6D /* reset to default font */ > > > Did you rebuild a distribution like Debian to verify that these values > > and structure are not used anywhere? > > I just did. https://codesearch.debian.net lists packages to check: > aalib > busybox > cde > console-cyrillic > dietlibc > doclifter > emscripten > fpc > gargoyle-free > gcc-10 > gcc-11 > gcc-12 > gcc-9 > gcc-arm-none-eabi > gcc-avr > gcc-riscv64-unknown-elf > gcc-snapshot > hurd > kbd > kfreebsd-10 > lazarus > libabigail > libexplain > linux > llvm-toolchain-11 > llvm-toolchain-12 > llvm-toolchain-13 > llvm-toolchain-14 > llvm-toolchain-9 > llvm-toolchain-snapshot > lynx > manpages-l10n > sequeler > strace > stress-ng > systemd > trinity > vala > valgrind > > Of these, I've ignored the kernels, and built only the latest released > version of a compiler. > > .--====[ aalib ] > aalinux.c: In function ‘linux_init’: > aalinux.c:106:34: error: storage size of ‘desc’ isn’t known > 106 | struct consolefontdesc desc; > | ^~~~ > aalinux.c:109:26: error: ‘GIO_FONTX’ undeclared (first use in this function) > 109 | i = ioctl (fd, GIO_FONTX, &desc); > | ^~~~~~~~~ > `---- > > .--====[ kbd ] > kdfontop.c: In function ‘kfont_restore_font’: > kdfontop.c:27:23: error: ‘PIO_FONTRESET’ undeclared (first use in this function) > 27 | if (ioctl(fd, PIO_FONTRESET, 0)) { > | ^~~~~~~~~~~~~ > kdfontop.c: In function ‘get_font_giofontx’: > kdfontop.c:89:32: error: storage size of ‘cfd’ isn’t known > kdfontop.c:102:30: error: ‘GIO_FONTX’ undeclared > kdfontop.c: In function ‘get_font_giofont’: > kdfontop.c:137:30: error: ‘GIO_FONT’ undeclared > kdfontop.c: In function ‘put_font_piofontx’: > kdfontop.c:242:32: error: storage size of ‘cfd’ isn’t known > kdfontop.c:262:31: error: ‘PIO_FONTX’ undeclared > kdfontop.c: In function ‘put_font_piofont’: > kdfontop.c:299:31: error: ‘PIO_FONT’ undeclared > `---- > > .--====[ gcc-12 ] > ../../../../src/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cpp:850:29: error: 'GIO_FONT' was not declared in this scope > 850 | unsigned IOCTL_GIO_FONT = GIO_FONT; > | ^~~~~~~~ > `---- > > .--====[ llvm-toolchain-14 ] > /<<PKGBUILDDIR>>/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp:865:29: error: use of undeclared identifier 'GIO_FONT' > unsigned IOCTL_GIO_FONT = GIO_FONT; > ^ > /<<PKGBUILDDIR>>/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp:898:29: error: use of undeclared identifier 'PIO_FONT' > unsigned IOCTL_PIO_FONT = PIO_FONT; > ^ > `---- > > .--====[ libexplain ] > libexplain/buffer/consolefontdesc.c: In function ‘explain_buffer_consolefontdesc’: > libexplain/buffer/consolefontdesc.c:34:48: error: invalid application of ‘sizeof’ to incomplete type ‘c > onst struct consolefontdesc’ > 34 | if (explain_is_efault_pointer(value, sizeof(*value))) > | ^ > libexplain/buffer/consolefontdesc.c:43:14: error: invalid use of undefined type ‘const struct consolefo > ntdesc’ > 43 | value->charcount, > | ^~ > `---- > > .--====[ trinity ] > In file included from ioctls/vt.c:8: > ioctls/vt.c:48:15: error: ‘GIO_FONT’ undeclared here (not in a function) > 48 | IOCTL(GIO_FONT), > | ^~~~~~~~ > include/ioctls.h:53:22: note: in definition of macro ‘IOCTL’ > 53 | { .request = _request, .name = #_request, } > | ^~~~~~~~ > ioctls/vt.c:49:15: error: ‘PIO_FONT’ undeclared here (not in a function) > 49 | IOCTL(PIO_FONT), > | ^~~~~~~~ > ioctls/vt.c:50:15: error: ‘GIO_FONTX’ undeclared here (not in a function) > ioctls/vt.c:51:15: error: ‘PIO_FONTX’ undeclared here (not in a function) > ioctls/vt.c:52:15: error: ‘PIO_FONTRESET’ undeclared here (not in a function) > `---- > > > So there _are_ failures in userspace. > But let's take a look at them: > > * aalib: dead upstream since 2001, this code is broken > * kbd: uses KDFONTOP, falls back if unsupported > * libsanitizer (bundled in gcc, llvm-toolchain): can drop > * libexplain: can drop > * trinity: can drop > > So we have one real failure, one fallback, and a few "lists all ioctls". > I'd say that's acceptable. Especially because presence of code that list > all ioctls means we would never done any clean-up otherwise. Nope, we can't break exiting tools, especially for things like gcc or llvm which do get built often. We've been down this path before, and had to revert things in the past. We can only remove stuff like this when all users are gone. Please work with the gcc and llvm developers to remove those usages, along with the other packages and maybe we can revisit this in 5 years or so. thanks, greg k-h
diff --git a/include/uapi/linux/kd.h b/include/uapi/linux/kd.h index ee929ece4112..d739b17fc942 100644 --- a/include/uapi/linux/kd.h +++ b/include/uapi/linux/kd.h @@ -6,19 +6,6 @@ /* 0x4B is 'K', to avoid collision with termios and vt */ -#define GIO_FONT 0x4B60 /* gets font in expanded form */ -#define PIO_FONT 0x4B61 /* use font in expanded form */ - -#define GIO_FONTX 0x4B6B /* get font using struct consolefontdesc */ -#define PIO_FONTX 0x4B6C /* set font using struct consolefontdesc */ -struct consolefontdesc { - unsigned short charcount; /* characters in font (256 or 512) */ - unsigned short charheight; /* scan lines per character (1-32) */ - char __user *chardata; /* font data in expanded form */ -}; - -#define PIO_FONTRESET 0x4B6D /* reset to default font */ - #define GIO_CMAP 0x4B70 /* gets colour palette on VGA+ */ #define PIO_CMAP 0x4B71 /* sets colour palette on VGA+ */
As was demonstrated by commit ff2047fb755d ("vt: drop old FONT ioctls"), old font ioctls like PIO_FONT have been deleted and KDFONTOP ioctl is used for years instead. However, unused definitions of these ioctl numbers and "strut consolefontdesc" still exist in a uapi header. They could have been removed since no userspace was using them. Otherwise they will become a misleading for users, and users will fail with ENOTTY with wrong call. We are moving old font ioctl numbers definitions here to guide users to use KDFONTOP. Signed-off-by: Xingrui Yi <yixingrui@linux.alibaba.com> --- include/uapi/linux/kd.h | 13 ------------- 1 file changed, 13 deletions(-)