Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.bugs.dist > #1054536 > unrolled thread

Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected"

Back to article view | Back to linux.debian.bugs.dist

Contents

  Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected" Marc Haber <mh+debian-packages@zugschlus.de> - 2021-04-03 15:10 +0200
    Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected" Bernhard Übelacker <bernhardu@mailbox.org> - 2021-04-04 15:10 +0200
      Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected" Chris Hofstaedtler <zeha@debian.org> - 2021-04-04 22:40 +0200
        Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected" Marc Haber <mh+debian-bugs@zugschlus.de> - 2021-04-04 23:00 +0200
          Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected" Chris Hofstaedtler <zeha@debian.org> - 2021-04-05 01:00 +0200
            Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected" "Theodore Ts'o" <tytso@mit.edu> - 2021-04-05 23:40 +0200
              Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected" Guillem Jover <guillem@hadrons.org> - 2021-04-24 17:00 +0200
                Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected" Chris Hofstaedtler <zeha@debian.org> - 2021-04-24 21:40 +0200
        Bug#986332: lsattr on certiain files in /dev results in "stack smashing detected" Bernhard Übelacker <bernhardu@mailbox.org> - 2021-04-04 23:10 +0200

#1054536 — 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  libblkid1    2.36.1-7
ii  libc6        2.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  <none>

Versions of packages e2fsprogs suggests:
pn  e2fsck-static  <none>
pn  fuse2fs        <none>
pn  gpart          <none>
ii  parted         3.4-1

-- no debconf information

[toc] | [next] | [standalone]

#1054596

[Multipart message — attachments visible in raw view] — view raw

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 *) 0xf759b62c00000000
     0x00007ffff7ec0cc7 in ioctl () at ../sysdeps/unix/syscall-template.S:120
     120     ../sysdeps/unix/syscall-template.S: Datei oder Verzeichnis nicht gefunden.
     1: x/i $pc
     => 0x7ffff7ec0cc7 <ioctl+7>:    cmp    $0xfffffffffffff001,%rax
     (gdb) bt
     #0  0x00007ffff7ec0cc7 in ioctl () at ../sysdeps/unix/syscall-template.S:120
     #1  0x00007ffff7fbcb17 in fgetflags (name=name@entry=0x7fffffffe83f "/dev/dri/card0", flags=flags@entry=0x7fffffffe3e0) at ../../../../lib/e2p/fgetflags.c:90
     #2  0x00005555555554d5 in list_attributes (name=name@entry=0x7fffffffe83f "/dev/dri/card0") at ../../../misc/lsattr.c:85
     #3  0x00005555555556c9 in lsattr_args (name=0x7fffffffe83f "/dev/dri/card0") at ../../../misc/lsattr.c:134
     #4  0x0000555555555369 in main (argc=<optimized out>, argv=<optimized out>) 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

[toc] | [prev] | [next] | [standalone]

#1054630

Hello Bernhard, Marc,

* Bernhard Übelacker <bernhardu@mailbox.org> [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

[toc] | [prev] | [next] | [standalone]

#1054632

Hi Chris,

On Sun, Apr 04, 2021 at 10:33:46PM +0200, Chris Hofstaedtler wrote:
> * Bernhard Übelacker <bernhardu@mailbox.org> [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: 0        Namelen: 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

[toc] | [prev] | [next] | [standalone]

#1054646

Hi Marc,

thanks for the followup.

* Marc Haber <mh+debian-bugs@zugschlus.de> [210404 22:03]:
> On Sun, Apr 04, 2021 at 10:33:46PM +0200, Chris Hofstaedtler wrote:
> > * Bernhard Übelacker <bernhardu@mailbox.org> [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: 0        Namelen: 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

[toc] | [prev] | [next] | [standalone]

#1054749

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 <unistd.h>
#include <stdio.h>

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

[toc] | [prev] | [next] | [standalone]

#1056839

[ 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

[toc] | [prev] | [next] | [standalone]

#1056862

* Guillem Jover <guillem@hadrons.org> [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

[toc] | [prev] | [next] | [standalone]

#1054634

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=2018888,mode=755)
...


Kind regards,
Bernhard

[toc] | [prev] | [standalone]

Back to top | Article view | linux.debian.bugs.dist

csiph-web