2002-02-07 09:07:43

by Andrey Panin

[permalink] [raw]
Subject: [PATCH] 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.

--
Andrey Panin | Embedded systems software engineer
[email protected] | PGP key: wwwkeys.eu.pgp.net


Attachments:
(No filename) (0.00 B)
(No filename) (232.00 B)
Download all attachments

2002-02-07 16:45:17

by Patrick Mochel

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


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