[9fans] devdraw memory corruption

[9fans] devdraw memory corruption

[cinap_lenrek]

when user does read of exactly 12*12 bytes on draw
ctl file, the snprint() adds one more \0 byte writing
beyond the user buffer and corrupting memory.

fix this by not snprint()ing the final space and add
it manually:

--- /sys/src/9/port/devdraw.c	Wed Dec 25 13:55:16 2013 UTC
+++ /sys/src/9/port/devdraw.c	Mon Jan 13 23:22:13 2014 UTC
@@ -1187,10 +1187,11 @@
  				error(Enodrawimage);
  			i = di->image;
  		}
-		n =
sprint(a, "%11d %11d %11s %11d %11d %11d %11d %11d %11d %11d %11d %11d ",
+		n =
sprint(a, "%11d %11d %11s %11d %11d %11d %11d %11d %11d %11d %11d %11d",
  			cl->clientid, cl->infoid, chantostr(buf, i->chan),
(i->flags&Frepl)==Frepl,
  			i->r.min.x, i->r.min.y, i->r.max.x, i->r.max.y,
  			i->clipr.min.x, i->clipr.min.y, i->clipr.max.x, i->clipr.max.y);
+		((char*)a)[n++] = ' ';
  		cl->infoid = -1;
  		break;

test program:

#include <u.h>
#include <libc.h>
#include <draw.h>

void
main(int argc, char *argv[])
{
	char buf[12*12+1];

	buf[12*12] = 'X';
	if(read(0, buf, 12*12) != 12*12)
		sysfatal("read: %r");
	if(buf[12*12] != 'X')
		sysfatal("corrupt");
}

term% ./8.out </dev/draw/new
corrupt

--
cinap

Re: [9fans] devdraw memory corruption

[cinap_lenrek]

just saw, sources seems to have already fixed this
by using snprint()...

so never mind :)

--
cinap

Re: [9fans] devdraw memory corruption

[erik quanstrom]

> +++ /sys/src/9/port/devdraw.c	Mon Jan 13 23:22:13 2014 UTC
> @@ -1187,10 +1187,11 @@
>   				error(Enodrawimage);
>   			i = di->image;
>   		}
> -		n =
> sprint(a, "%11d %11d %11s %11d %11d %11d %11d %11d %11d %11d %11d %11d ",
> +		n =
> sprint(a, "%11d %11d %11s %11d %11d %11d %11d %11d %11d %11d %11d %11d",
>   			cl->clientid, cl->infoid, chantostr(buf, i->chan),
> (i->flags&Frepl)==Frepl,
>   			i->r.min.x, i->r.min.y, i->r.max.x, i->r.max.y,
>   			i->clipr.min.x, i->clipr.min.y, i->clipr.max.x, i->clipr.max.y);
> +		((char*)a)[n++] = ' ';
>   		cl->infoid = -1;
>   		break;

why not use a 145 byte buffer and readstr?

- erik

Re: [9fans] devdraw memory corruption

[dexen deVries]

[-- Attachment #1: Type: text/plain, Size: 652 bytes --]

On Tuesday 14 of January 2014 00:44:47 cinap_lenrek@felloff.net wrote:
> when user does read of exactly 12*12 bytes on draw
> ctl file, the snprint() adds one more \0 byte writing
> beyond the user buffer and corrupting memory.

what symptoms were you getting?

i'm seeing rare, seemingly random glitches in plan9port Acme.

some windows get only partially re-drawn -- any combination of borders, 
background and text can be missing, as per attachment. the top right window 
should be showing file list, but only background was drawn.


i've ported this patch to p9p to see if the breakage stops.

-- 
dexen deVries

[[[↓][→]]]

[-- Attachment #2: borken-acme.png --]
[-- Type: image/png, Size: 28357 bytes --]