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] [day] [month] [year] [list]
Message-ID: <YuUdWoa7UFHmkNu9@kroah.com>
Date:   Sat, 30 Jul 2022 14:00:26 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Adam Borowski <kilobyte@...band.pl>
Cc:     Xingrui Yi <yixingrui@...ux.alibaba.com>,
        linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] vt: remove old FONT ioctls definitions in uapi

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ