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
