[PATCH] read() from driverfs files can read more bytes then requested

[PATCH] read() from driverfs files can read more bytes then requested

[Andrey Panin]

[-- Attachment #1.1: Type: text/plain, Size: 888 bytes --]

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.

-- 
Andrey Panin            | Embedded systems software engineer
pazke@orbita1.ru        | PGP key: wwwkeys.eu.pgp.net

[-- Attachment #1.2: patch-driverfs --]
[-- Type: text/plain, Size: 388 bytes --]

diff -urN -X /usr/dontdiff /linux.2.5.3-dj3/fs/driverfs/inode.c /linux/fs/driverfs/inode.c
--- /linux.2.5.3-dj3/fs/driverfs/inode.c	Wed Feb  6 23:42:06 2002
+++ /linux/fs/driverfs/inode.c	Wed Feb  6 23:34:05 2002
@@ -255,6 +255,9 @@
 
 		len = entry->show(dev,page,count,*ppos);
 
+		if (len > count)
+			len = count;
+
 		if (len <= 0) {
 			if (len < 0)
 				retval = len;

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: [PATCH] read() from driverfs files can read more bytes then requested

[Patrick Mochel]

On Thu, 7 Feb 2002, Andrey Panin wrote:

> 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 :)

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