read() from driverfs files can read more bytes then requested
PATCH 1/1 read() from driverfs files can read more bytes then requested
Hi all,
small program below crashes on read() from driverfs file:
int main(void)
{
int fd, ret;
char buf[16];
fd = open("/var/driver/root/pci0/status", 0);
ret = read(fd, buf, sizeof(buf));
close(fd);
}it's because driverfs_read_file() function blindly uses entry->show() return value without sanity check. As a result userspace process requested 16 bytes, but got ~45 and smashed stack as a bonus. You can also get this effect pressing F3 in Midnight Commander on driverfs files.
Attached patch adds check that returned value is less then requested byte count. I know that actual callback function device_read_status() should also be fixed, but I found this bug after midnight and decided to sleep a little :)
Best regards.
Signature · 2 lines
Andrey Panin | Embedded systems software engineer [email protected] | PGP key: wwwkeys.eu.pgp.net
linux/fs/driverfs/inode.c renamed from linux.2.5.3-dj3/fs/driverfs/inode.c +3 -0
@@ -255,6 +255,9 @@ len = entry->show (dev,page,count,*ppos); +if (len > count) + len = count; +if (len <=0 ) {if (len <0 ) retval = len;
Quoted 19 lines · Andrey Panin
Hi all,
small program below crashes on
read()from driverfs file:int main(void)
{ int fd, ret; char buf[16]; fd = open("/var/driver/root/pci0/status", 0); ret = read(fd, buf, sizeof(buf)); close(fd); }it's because
driverfs_read_file()function blindly usesentry->show()return value without sanity check. As a result userspace process requested 16 bytes, but got ~45 and smashed stack as a bonus. You can also get this effect pressing F3 in Midnight Commander on driverfs files.
Quoted 4 lines
Attached patch adds check that returned value is less then requested byte count. I know that actual callback function
device_read_status()should also be fixed, but I found this bug after midnight and decided to sleep a little :)
That sanity check was in there, once upon a time. However, in moving the weight from the driver callbacks to the driverfs read_file() and write_file(), it must have got dropped...
Thank you. It's been applied and will be pushed forward.
-pat