Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected"
tags 986332 +pending
thanks
The following commit should fix things; it will be in the next release
of e2fsprogs.
- Ted
commit a3f844da91f0c01209a5d778a5af57fabe245332
Author: Theodore Ts'o
Date: Fri Jul 16 22:31:26 2021 -0400
libe2p: use stat to prevent calling EXT2_IOC_[GS]ETFLAGS on devices
Some devices can react badly to the EXT2_IOC_[GS]ETFLAGS ioctls, since
ioctl codes are not guaranteed to be unique across different device
drivers and file systems.
Addresses-Debian-Bug: #986332
Signed-off-by: Theodore Ts'o
diff --git a/lib/e2p/fgetflags.c b/lib/e2p/fgetflags.c
index 93e130c6..24a7166b 100644
--- a/lib/e2p/fgetflags.c
+++ b/lib/e2p/fgetflags.c
@@ -79,14 +79,26 @@ int fgetflags (const char * name, unsigned long * flags)
*flags = f;
return (save_errno);
#elif HAVE_EXT2_IOCTLS
+ struct stat buf;
int fd, r, f, save_errno = 0;
+ if (!stat(name, &buf) &&
+ !S_ISREG(buf.st_mode) && !S_ISDIR(buf.st_mode)) {
+ errno = EOPNOTSUPP;
+ return -1;
+ }
fd = open(name, OPEN_FLAGS);
if (fd == -1) {
if (errno == ELOOP || errno == ENXIO)
errno = EOPNOTSUPP;
return -1;
}
+ if (!fstat(fd, &buf) &&
+ !S_ISREG(buf.st_mode) && !S_ISDIR(buf.st_mode)) {
+ close(fd);
+ errno = EOPNOTSUPP;
+ return -1;
+ }
r = ioctl(fd, EXT2_IOC_GETFLAGS, &f);
if (r == -1) {
if (errno == ENOTTY)
diff --git a/lib/e2p/fsetflags.c b/lib/e2p/fsetflags.c
index 6455e386..d865d243 100644
--- a/lib/e2p/fsetflags.c
+++ b/lib/e2p/fsetflags.c
@@ -80,14 +80,26 @@ int fsetflags (const char * name, unsigned long flags)
int f = (int) flags;
return syscall(SYS_fsctl, name, EXT2_IOC_SETFLAGS, &f, 0);
#elif HAVE_EXT2_IOCTLS
+ struct stat buf;
int fd, r, f, save_errno = 0;
+ if (!stat(name, &buf) &&
+ !S_ISREG(buf.st_mode) && !S_ISDIR(buf.st_mode)) {
+ errno = EOPNOTSUPP;
+ return -1;
+ }
fd = open(name, OPEN_FLAGS);
if (fd == -1) {
if (errno == ELOOP || errno == ENXIO)
errno = EOPNOTSUPP;
return -1;
}
+ if (!fstat(fd, &buf) &&
+ !S_ISREG(buf.st_mode) && !S_ISDIR(buf.st_mode)) {
+ close(fd);
+ errno = EOPNOTSUPP;
+ return -1;
+ }
f = (int) flags;
r = ioctl(fd, EXT2_IOC_SETFLAGS, &f);
if (r == -1) {
Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected"
* Guillem Jover [210424 16:53]: > So, while I was trying to debug this in aide on my server, after > having added few aide ignore entries for specific devices, at some > point while within gdb, I suddenly lost the ssh connection as the > server suddenly rebooted, for which I was very puzzled, and paranoid! I tried to reproduce this, and stumbled upon the watchdog and watchdog0 files. If they are the problem, well... echo > /dev/watchdog sleep 30 # notice your machine is rebooting That would probably by design. I guess its well understood that writing to random "files" in /dev is a bad plan; it appears -opening- then is, too. Chris
Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected"
[ Just upgraded my server to bullseye, and aide started to fail, now noticed this report, after also hitting the error via lsattr. ] On Mon, 2021-04-05 at 17:35:54 -0400, Theodore Ts'o wrote: > On Mon, Apr 05, 2021 at 12:46:48AM +0200, Chris Hofstaedtler wrote: > > AFAICT, for /dev/dri/card0 the ioctl ends up in the kernel's > > drm_ioctl [2], which will blindly call copy_to_user assuming the > > output size is the same as the input size (8 bytes). This is wrong > > for FS_IOC_GETFLAGS, at least for normal files. > > > > Maybe the best thing is to put the lstat check back in? > > Or maybe lsattr should expect that the kernel might actually use the > > 8 bytes? I have checked various fs ioctl functions, and they all > > seem to return 4 bytes, except for orangefs [3] ... > > What's going on is that apparently there is an overlap between the > ioctl code FS_IOC_GETFLAGS (aka EXT2_IOC_GETFLAGS) and some ioctl code > used by device driver responding to /dev/dri/card0, in drm_ioctl. I > had the vague thought that at some point, we might be able to set and > get file system flags on device files, which is why I removed the > lstat check. I wasn't counting on the fact that there would be ioctl > code collisions --- which in retrospect, was hopelessly optimistic on > my part. So, while I was trying to debug this in aide on my server, after having added few aide ignore entries for specific devices, at some point while within gdb, I suddenly lost the ssh connection as the server suddenly rebooted, for which I was very puzzled, and paranoid! What you mention above, of colliding IOCTL codes would actually explain the cause of the reboot, if it ends up hitting an IOCTL setting some wrong state somewhere. I also noticed this being an issue with at least /dev/kfd, but I'm not very eager to try this again there to determine what other devices this might affect, if this can result in sending random commands to random devices on the live server. :/ > So yeah, we need to put the lstat check back in. > Fortunately, the fortify compile option detectsd the stack smash, so > it's not critical that we get this fixed ASAP, but we ultimately do > need to put the lstat check back in. I'd appreciate if this could be done sooner rather than later, and it seems to me it probably deserves a higher severity. (I've for now disabled aide to avoid any crashes or strange behavior.) Thanks, Guillem
Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected"
On Mon, Apr 05, 2021 at 12:46:48AM +0200, Chris Hofstaedtler wrote:
>
> AFAICT, for /dev/dri/card0 the ioctl ends up in the kernel's
> drm_ioctl [2], which will blindly call copy_to_user assuming the
> output size is the same as the input size (8 bytes). This is wrong
> for FS_IOC_GETFLAGS, at least for normal files.
>
> Maybe the best thing is to put the lstat check back in?
> Or maybe lsattr should expect that the kernel might actually use the
> 8 bytes? I have checked various fs ioctl functions, and they all
> seem to return 4 bytes, except for orangefs [3] ...
What's going on is that apparently there is an overlap between the
ioctl code FS_IOC_GETFLAGS (aka EXT2_IOC_GETFLAGS) and some ioctl code
used by device driver responding to /dev/dri/card0, in drm_ioctl. I
had the vague thought that at some point, we might be able to set and
get file system flags on device files, which is why I removed the
lstat check. I wasn't counting on the fact that there would be ioctl
code collisions --- which in retrospect, was hopelessly optimistic on
my part.
So yeah, we need to put the lstat check back in.
I checked fs/orange/file.c and it is also using 4 bytes (int is always
32 bits even on 64-bit platforms):
if (cmd == FS_IOC_GETFLAGS) {
ret = orangefs_getflags(inode, &uval);
if (ret)
return ret;
gossip_debug(GOSSIP_FILE_DEBUG,
"orangefs_ioctl: FS_IOC_GETFLAGS: %llu\n",
(unsigned long long)uval);
return put_user(uval, (int __user *)arg);
^
% cat /tmp/foo.c
#include
#include
int main(int argc, char **argv)
{
printf("size of int: %d\n", sizeof(int));
return 0;
}
% cc -o /tmp/foo /tmp/foo.c
% /tmp/foo
size of int: 4
Fortunately, the fortify compile option detectsd the stack smash, so
it's not critical that we get this fixed ASAP, but we ultimately do
need to put the lstat check back in.
- Ted
Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected"
Hello Chris, Am 04.04.21 um 22:33 schrieb Chris Hofstaedtler: Hello Bernhard, Marc, Some more questions: 1) which kernel version is this? My test was just inside a temporary VM with current testing. But I can still reproduce this with current testing kernel at the host too: Linux rechner 5.10.0-5-amd64 #1 SMP Debian 5.10.24-1 (2021-03-19) x86_64 GNU/Linux 2) /dev/dri is on tmpfs? bernhard@rechner:~$ mount udev on /dev type devtmpfs (rw,nosuid,noexec,relatime,size=8075552k,nr_inodes=201,mode=755) ... Kind regards, Bernhard
Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected"
Hi Marc, thanks for the followup. * Marc Haber [210404 22:03]: > On Sun, Apr 04, 2021 at 10:33:46PM +0200, Chris Hofstaedtler wrote: > > * Bernhard Übelacker [210404 20:32]: > > > Dear Maintainer, > > > tried to locate the exact smashing. > > > It looks like the ioctl(EXT2_IOC_GETFLAGS) takes an int* parameter, > > > but writes 8 bytes instead of just sizeof(int) to the given address. > > > > Some more questions: > > 1) which kernel version is this? > > 2) /dev/dri is on tmpfs? > > 1 [2/4021]mh@testsid85:~ $ sudo lsattr /dev/dri/card0 > [sudo] password for mh: > *** stack smashing detected ***: terminated > Aborted > 134 [3/4022]mh@testsid85:~ $ uname -a > Linux testsid85 5.10.0-5-amd64 #1 SMP Debian 5.10.26-1 (2021-03-27) x86_64 > GNU/Linux > [4/4023]mh@testsid85:~ $ stat -f /dev/dri > File: "/dev/dri" > ID: 0Namelen: 255 Type: tmpfs > Block size: 4096 Fundamental block size: 4096 > Blocks: Total: 40336 Free: 40336 Available: 40336 > Inodes: Total: 40336 Free: 39600 > [5/4024]mh@testsid85:~ $ > > Other /dev device nods can be lsattr'd without error. I was wondering about changes since buster, and indeed: Upstream commit 40ea4628 [1] removes the lstat call which shielded the ioctl call later on. On buster, lsattr /dev/dri/card0 just gives: lsattr: Operation not supported While reading flags on /dev/dri/card0 (Even with Linux 5.10.0-0.bpo.3-amd64.) Now, for the actual issue: AFAICT, for /dev/dri/card0 the ioctl ends up in the kernel's drm_ioctl [2], which will blindly call copy_to_user assuming the output size is the same as the input size (8 bytes). This is wrong for FS_IOC_GETFLAGS, at least for normal files. Maybe the best thing is to put the lstat check back in? Or maybe lsattr should expect that the kernel might actually use the 8 bytes? I have checked various fs ioctl functions, and they all seem to return 4 bytes, except for orangefs [3] ... Chris [1] https://git.kernel.org/pub/scm/fs/ext2/e2fsprogs.git/commit/lib/e2p/fgetflags.c?id=40ea4628ba1b55f8eba311f12399d039698dbeeb [2] https://elixir.bootlin.com/linux/v5.10.27/source/drivers/gpu/drm/drm_ioctl.c#L888 [3] https://elixir.bootlin.com/linux/v5.10.27/source/fs/orangefs/file.c#L378
Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected"
Hi Chris, On Sun, Apr 04, 2021 at 10:33:46PM +0200, Chris Hofstaedtler wrote: > * Bernhard Übelacker [210404 20:32]: > > Dear Maintainer, > > tried to locate the exact smashing. > > It looks like the ioctl(EXT2_IOC_GETFLAGS) takes an int* parameter, > > but writes 8 bytes instead of just sizeof(int) to the given address. > > Some more questions: > 1) which kernel version is this? > 2) /dev/dri is on tmpfs? 1 [2/4021]mh@testsid85:~ $ sudo lsattr /dev/dri/card0 [sudo] password for mh: *** stack smashing detected ***: terminated Aborted 134 [3/4022]mh@testsid85:~ $ uname -a Linux testsid85 5.10.0-5-amd64 #1 SMP Debian 5.10.26-1 (2021-03-27) x86_64 GNU/Linux [4/4023]mh@testsid85:~ $ stat -f /dev/dri File: "/dev/dri" ID: 0Namelen: 255 Type: tmpfs Block size: 4096 Fundamental block size: 4096 Blocks: Total: 40336 Free: 40336 Available: 40336 Inodes: Total: 40336 Free: 39600 [5/4024]mh@testsid85:~ $ Other /dev device nods can be lsattr'd without error. Greetings Marc -- - Marc Haber | "I don't trust Computers. They | Mailadresse im Header Leimen, Germany| lose things."Winona Ryder | Fon: *49 6224 1600402 Nordisch by Nature | How to make an American Quilt | Fax: *49 6224 1600421
Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected"
Hello Bernhard, Marc, * Bernhard Übelacker [210404 20:32]: > Dear Maintainer, > tried to locate the exact smashing. > It looks like the ioctl(EXT2_IOC_GETFLAGS) takes an int* parameter, > but writes 8 bytes instead of just sizeof(int) to the given address. Some more questions: 1) which kernel version is this? 2) /dev/dri is on tmpfs? Chris
Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected"
Dear Maintainer, tried to locate the exact smashing. It looks like the ioctl(EXT2_IOC_GETFLAGS) takes an int* parameter, but writes 8 bytes instead of just sizeof(int) to the given address. Kind regards, Bernhard Old value = (void *) 0xf759b62c03711000 New value = (void *) 0xf759b62c 0x77ec0cc7 in ioctl () at ../sysdeps/unix/syscall-template.S:120 120 ../sysdeps/unix/syscall-template.S: Datei oder Verzeichnis nicht gefunden. 1: x/i $pc => 0x77ec0cc7 :cmp$0xf001,%rax (gdb) bt #0 0x77ec0cc7 in ioctl () at ../sysdeps/unix/syscall-template.S:120 #1 0x77fbcb17 in fgetflags (name=name@entry=0x7fffe83f "/dev/dri/card0", flags=flags@entry=0x7fffe3e0) at ../../../../lib/e2p/fgetflags.c:90 #2 0x54d5 in list_attributes (name=name@entry=0x7fffe83f "/dev/dri/card0") at ../../../misc/lsattr.c:85 #3 0x56c9 in lsattr_args (name=0x7fffe83f "/dev/dri/card0") at ../../../misc/lsattr.c:134 #4 0x5369 in main (argc=, argv=) at ../../../misc/lsattr.c:221 https://sources.debian.org/src/e2fsprogs/1.46.2-1/lib/e2p/fgetflags.c/#L90 https://git.kernel.org/pub/scm/fs/ext2/e2fsprogs.git/tree/lib/e2p/fgetflags.c#n90 # single-use Bullseye/testing amd64 qemu VM 2021-04-04 echo "set enable-bracketed-paste off" >> /etc/inputrc; bash apt update # to speedup testing mv /etc/manpath.config /etc/manpath.config.renamed apt install libeatmydata1 export LD_PRELOAD=/usr/lib/x86_64-linux-gnu/libeatmydata.so apt dist-upgrade apt install systemd-coredump gdb valgrind \ e2fsprogs-dbgsym libext2fs2-dbgsym . benutzer@debian:~$ lsattr /dev/dri/card0 *** stack smashing detected ***: terminated Abgebrochen (Speicherabzug geschrieben) root@debian:~# coredumpctl list TIMEPID UID GID SIG COREFILE EXE Sun 2021-04-04 14:22:59 CEST 1921 1000 1000 6 present /usr/bin/lsattr root@debian:~# coredumpctl gdb 1921 PID: 1921 (lsattr) UID: 1000 (benutzer) GID: 1000 (benutzer) Signal: 6 (ABRT) Timestamp: Sun 2021-04-04 14:22:59 CEST (50s ago) Command Line: lsattr /dev/dri/card0 Executable: /usr/bin/lsattr Control Group: /user.slice/user-1000.slice/session-3.scope Unit: session-3.scope Slice: user-1000.slice Session: 3 Owner UID: 1000 (benutzer) Boot ID: de580d9e15564f17b195ec068c7129dc Machine ID: 33f18f39d2a9438eb75b0ed52848afcd Hostname: debian Storage: /var/lib/systemd/coredump/core.lsattr.1000.de580d9e15564f17b195ec068c7129dc.1921.161753897900.zst Message: Process 1921 (lsattr) of user 1000 dumped core. Stack trace of thread 1921: #0 0x7f7ea4286ce1 __GI_raise (libc.so.6 + 0x3bce1) #1 0x7f7ea4270537 __GI_abort (libc.so.6 + 0x25537) #2 0x7f7ea42c9768 __libc_message (libc.so.6 + 0x7e768) #3 0x7f7ea4358652 __GI___fortify_fail (libc.so.6 + 0x10d652) #4 0x7f7ea4358630 __stack_chk_fail (libc.so.6 + 0x10d630) #5 0x7f7ea443bbd6 fgetflags (libe2p.so.2 + 0x3bd6) #6 0x557d54ea24d5 n/a (lsattr + 0x14d5) #7 0x557d54ea26c9 n/a (lsattr + 0x16c9) #8 0x557d54ea2369 n/a (lsattr + 0x1369) #9 0x7f7ea4271d0a __libc_start_main (libc.so.6 + 0x26d0a) #10 0x557d54ea23ea n/a (lsattr + 0x13ea) ... Core was generated by `lsattr /dev/dri/card0'. Program terminated with signal SIGABRT, Aborted. #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 50 ../sysdeps/unix/sysv/linux/raise.c: Datei oder Verzeichnis nicht gefunden. (gdb) bt #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 #1 0x7f7ea4270537 in __GI_abort () at abort.c:79 #2 0x7f7ea42c9768 in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x7f7ea43d7c24 "*** %s ***: terminated\n") at ../sysdeps/posix/libc_fatal.c:155 #3 0x7f7ea4358652 in __GI___fortify_fail (msg=msg@entry=0x7f7ea43d7c0c "stack smashing detected") at fortify_fail.c:26 #4 0x7f7ea4358630 in __stack_chk_fail () at stack_chk_fail.c:24 #5 0x7f7ea443bbd6 in fgetflags () from /lib/x86_64-linux-gnu/libe2p.so.2 #6 0x557d54ea24d5 in ?? () #7 0x557d54ea26c9 in ?? () #8 0x557d54ea2369 in ?? () #9 0x7f7ea4271d0a in __libc_start_main (main=0x557d54ea21d0, argc=2, argv=0x7ffda1e5c978, init=, fini=, rtld_fini=, stack_end=0x7ffda1e5c968) at ../csu/libc-start.c:308 #10 0x557d54ea23ea in ?? () (gdb) bt #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 #1 0x7f7ea4270537 in __GI_abort () at abort.c:79 #2 0x7f7ea42c9768 in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x7f7ea43d7c24 "*** %s ***: terminated\n") at ../
Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected"
Package: e2fsprogs Version: 1.46.2-1 Severity: normal File: /usr/bin/lsattr Hi, while debugging aide, I have stumbled upon a possible bug in lsattr or one of the underlying libraries. Doing lsattr on /dev/dri/card0 results in an abort with "stack smashing detected": | root@testsid85:~# sudo lsattr /dev/dri/card0 | *** stack smashing detected ***: terminated | Aborted Similiar behavior is observed when aide is trying to read the ext2 attrs of this file, so this is most probably a library issue. I can provide a backtrace if it helps. Greetings Marc -- System Information: Debian Release: bullseye/sid APT prefers unstable APT policy: (500, 'unstable'), (500, 'testing'), (500, 'stable') Architecture: amd64 (x86_64) Kernel: Linux 5.10.0-5-amd64 (SMP w/2 CPU threads) Locale: LANG=de_DE.utf8, LC_CTYPE=de_DE.utf8 (charmap=UTF-8), LANGUAGE=en Shell: /bin/sh linked to /usr/bin/dash Init: systemd (via /run/systemd/system) LSM: AppArmor: enabled Versions of packages e2fsprogs depends on: ii libblkid12.36.1-7 ii libc62.31-11 ii libcom-err2 1.46.2-1 ii libext2fs2 1.46.2-1 ii libss2 1.46.2-1 ii libuuid1 2.36.1-7 ii logsave 1.46.2-1 Versions of packages e2fsprogs recommends: pn e2fsprogs-l10n Versions of packages e2fsprogs suggests: pn e2fsck-static pn fuse2fs pn gpart ii parted 3.4-1 -- no debconf information
