Hi! > The value av7110->debi_virt is stored in DMA memory, and it is assigned > to data, and thus data[0] can be modified at any time by malicious > hardware. In this case, "if (data[0] < 2)" can be passed, but then > data[0] can be changed into a large number, which may cause buffer > overflow when the code "av7110->ci_slot[data[0]]" is used. > > To fix this possible bug, data[0] is assigned to a local variable, which > replaces the use of data[0].
I'm pretty sure hardware capable of manipulating memory can work
around any such checks, but...
> +++ b/drivers/media/pci/ttpci/av7110.c
> @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie)
> case DATA_CI_GET:
> {
> u8 *data = av7110->debi_virt;
> + u8 data_0 = data[0];
>
> - if ((data[0] < 2) && data[2] == 0xff) {
> + if (data_0 < 2 && data[2] == 0xff) {
> int flags = 0;
> if (data[5] > 0)
> flags |= CA_CI_MODULE_PRESENT;
> if (data[5] > 5)
> flags |= CA_CI_MODULE_READY;
> - av7110->ci_slot[data[0]].flags = flags;
> + av7110->ci_slot[data_0].flags = flags;
This does not even do what it says. Compiler is still free to access
data[0] multiple times. It needs READ_ONCE() to be effective.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: PGP signature
