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

Groups > linux.debian.bugs.dist > #1054749

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

Show all headers | View raw

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

Back to linux.debian.bugs.dist | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

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

csiph-web