shithub: front

Info  •  Files  •  Log  •  Branches

devuart: Fix memory leak when reading status file

Drivers where allocating a READSTR size buffer,
then readstr() it. But readstr() can raise an
error on pagefault, resulting in the buffer to
be leaked.

Instead, we change the interface and allocate
the buffer in devuart read handler, passing
the driver start and end pointers into it.

Also, provide a default implementation (when
status == nil), avoiding some duplication.

--- a/sys/src/9/arm64/uartqemu.c

+++ b/sys/src/9/arm64/uartqemu.c

@@ -282,33 +282,6 @@

 {

 }

-static long

-status(Uart *uart, void *buf, long n, long offset)

-{

-	char *p;

-

-	p = malloc(READSTR);

-	if(p == nil)

-		error(Enomem);

-	snprint(p, READSTR,

-		"b%d\n"

-		"dev(%d) type(%d) framing(%d) overruns(%d) "

-		"berr(%d) serr(%d)\n",

-

-		uart->baud,

-		uart->dev,

-		uart->type,

-		uart->ferr,

-		uart->oerr,

-		uart->berr,

-		uart->serr

-	);

-	n = readstr(offset, buf, n, p);

-	free(p);

-

-	return n;

-}

-

 static void

 donothing(Uart*, int)

 {

@@ -356,7 +329,6 @@

 	.modemctl	= donothing,

 	.rts		= rts,

 	.dtr		= donothing,

-	.status		= status,

 	.fifo		= donothing,

 	.getc		= getc,

 	.putc		= putc,

--- a/sys/src/9/bcm/uartmini.c

+++ b/sys/src/9/bcm/uartmini.c

@@ -225,33 +225,6 @@

 		ap[MuMcr] |= RtsN;

 }

-static long

-status(Uart *uart, void *buf, long n, long offset)

-{

-	char *p;

-

-	p = malloc(READSTR);

-	if(p == nil)

-		error(Enomem);

-	snprint(p, READSTR,

-		"b%d\n"

-		"dev(%d) type(%d) framing(%d) overruns(%d) "

-		"berr(%d) serr(%d)\n",

-

-		uart->baud,

-		uart->dev,

-		uart->type,

-		uart->ferr,

-		uart->oerr,

-		uart->berr,

-		uart->serr

-	);

-	n = readstr(offset, buf, n, p);

-	free(p);

-

-	return n;

-}

-

 static void

 donothing(Uart*, int)

 {

@@ -295,7 +268,6 @@

 	.modemctl	= donothing,

 	.rts		= rts,

 	.dtr		= donothing,

-	.status		= status,

 	.fifo		= donothing,

 	.getc		= getc,

 	.putc		= putc,

--- a/sys/src/9/bcm/uartpl011.c

+++ b/sys/src/9/bcm/uartpl011.c

@@ -282,33 +282,6 @@

 {

 }

-static long

-status(Uart *uart, void *buf, long n, long offset)

-{

-	char *p;

-

-	p = malloc(READSTR);

-	if(p == nil)

-		error(Enomem);

-	snprint(p, READSTR,

-		"b%d\n"

-		"dev(%d) type(%d) framing(%d) overruns(%d) "

-		"berr(%d) serr(%d)\n",

-

-		uart->baud,

-		uart->dev,

-		uart->type,

-		uart->ferr,

-		uart->oerr,

-		uart->berr,

-		uart->serr

-	);

-	n = readstr(offset, buf, n, p);

-	free(p);

-

-	return n;

-}

-

 static void

 donothing(Uart*, int)

 {

@@ -348,7 +321,6 @@

 	.modemctl	= donothing,

 	.rts		= rts,

 	.dtr		= donothing,

-	.status		= status,

 	.fifo		= donothing,

 	.getc		= getc,

 	.putc		= putc,

--- a/sys/src/9/imx8/uartimx.c

+++ b/sys/src/9/imx8/uartimx.c

@@ -273,33 +273,6 @@

 {

 }

-static long

-status(Uart *uart, void *buf, long n, long offset)

-{

-	char *p;

-

-	p = malloc(READSTR);

-	if(p == nil)

-		error(Enomem);

-	snprint(p, READSTR,

-		"b%d\n"

-		"dev(%d) type(%d) framing(%d) overruns(%d) "

-		"berr(%d) serr(%d)\n",

-

-		uart->baud,

-		uart->dev,

-		uart->type,

-		uart->ferr,

-		uart->oerr,

-		uart->berr,

-		uart->serr

-	);

-	n = readstr(offset, buf, n, p);

-	free(p);

-

-	return n;

-}

-

 static void

 interrupt(Ureg*, void *arg)

 {

@@ -393,7 +366,6 @@

 	.modemctl	= donothing,

 	.rts		= rts,

 	.dtr		= donothing,

-	.status		= status,

 	.fifo		= donothing,

 	.getc		= getc,

 	.putc		= putc,

--- a/sys/src/9/kw/uartkw.c

+++ b/sys/src/9/kw/uartkw.c

@@ -255,13 +255,6 @@

 	USED(uart, on);

 }

-static long

-kw_status(Uart* uart, void* buf, long n, long offset)

-{

-	USED(uart, buf, n, offset);

-	return 0;

-}

-

 static void

 kw_fifo(Uart* uart, int level)

 {

@@ -318,7 +311,6 @@

 	.modemctl	= kw_modemctl,

 	.rts		= kw_rts,

 	.dtr		= kw_dtr,

-	.status		= kw_status,

 	.fifo		= kw_fifo,

 	.getc		= kw_getc,

 	.putc		= kw_putc,

--- a/sys/src/9/lx2k/uartlx2k.c

+++ b/sys/src/9/lx2k/uartlx2k.c

@@ -284,33 +284,6 @@

 {

 }

-static long

-status(Uart *uart, void *buf, long n, long offset)

-{

-	char *p;

-

-	p = malloc(READSTR);

-	if(p == nil)

-		error(Enomem);

-	snprint(p, READSTR,

-		"b%d\n"

-		"dev(%d) type(%d) framing(%d) overruns(%d) "

-		"berr(%d) serr(%d)\n",

-

-		uart->baud,

-		uart->dev,

-		uart->type,

-		uart->ferr,

-		uart->oerr,

-		uart->berr,

-		uart->serr

-	);

-	n = readstr(offset, buf, n, p);

-	free(p);

-

-	return n;

-}

-

 static void

 donothing(Uart*, int)

 {

@@ -359,7 +332,6 @@

 	.modemctl	= donothing,

 	.rts		= rts,

 	.dtr		= donothing,

-	.status		= status,

 	.fifo		= donothing,

 	.getc		= getc,

 	.putc		= putc,

--- a/sys/src/9/mt7688/uarti8250.c

+++ b/sys/src/9/mt7688/uarti8250.c

@@ -145,20 +145,18 @@

 #define csr8w(c, r, v)	((c)->io[r] = (c)->sticky[r] | (v))

 #define csr8o(c, r, v)	((c)->io[r] = (v))

-static long

-i8250status(Uart* uart, void* buf, long n, long offset)

+static char*

+i8250status(Uart* uart, char *p, char *e)

 {

-	char *p;

 	Ctlr *ctlr;

 	uchar ier, lcr, mcr, msr;

 	ctlr = uart->regs;

-	p = malloc(READSTR);

 	mcr = ctlr->sticky[Mcr];

 	msr = csr8r(ctlr, Msr);

 	ier = ctlr->sticky[Ier];

 	lcr = ctlr->sticky[Lcr];

-	snprint(p, READSTR,

+	return seprint(p, e,

 		"b%d c%d d%d e%d l%d m%d p%c r%d s%d i%d\n"

 		"dev(%d) type(%d) framing(%d) overruns(%d) "

 		"berr(%d) serr(%d)%s%s%s%s\n",

@@ -185,10 +183,6 @@

 		(msr & Dcd) ? " dcd": "",

 		(msr & Ri) ? " ring": ""

 	);

-	n = readstr(offset, buf, n, p);

-	free(p);

-

-	return n;

 }

 static void

--- a/sys/src/9/mtx/uarti8250.c

+++ b/sys/src/9/mtx/uarti8250.c

@@ -149,20 +149,18 @@

 #define csr8r(c, r)	inb((c)->io+(r))

 #define csr8w(c, r, v)	outb((c)->io+(r), (c)->sticky[(r)]|(v))

-static long

-i8250status(Uart* uart, void* buf, long n, long offset)

+static char*

+i8250status(Uart* uart, char *p, char *e)

 {

-	char *p;

 	Ctlr *ctlr;

 	uchar ier, lcr, mcr, msr;

 	ctlr = uart->regs;

-	p = malloc(READSTR);

 	mcr = ctlr->sticky[Mcr];

 	msr = csr8r(ctlr, Msr);

 	ier = ctlr->sticky[Ier];

 	lcr = ctlr->sticky[Lcr];

-	snprint(p, READSTR,

+	return seprint(p, e,

 		"b%d c%d d%d e%d l%d m%d p%c r%d s%d i%d\n"

 		"dev(%d) type(%d) framing(%d) overruns(%d)%s%s%s%s\n",

@@ -186,10 +184,6 @@

 		(msr & Dcd) ? " dcd": "",

 		(msr & Ri) ? " ring": ""

 	);

-	n = readstr(offset, buf, n, p);

-	free(p);

-

-	return n;

 }

 static void

--- a/sys/src/9/omap/uarti8250.c

+++ b/sys/src/9/omap/uarti8250.c

@@ -174,20 +174,18 @@

 #define csr8w(c, r, v)	((c)->io[r] = (c)->sticky[r] | (v), coherence())

 #define csr8o(c, r, v)	((c)->io[r] = (v), coherence())

-static long

-i8250status(Uart* uart, void* buf, long n, long offset)

+static char*

+i8250status(Uart* uart, char *p, char *e)

 {

-	char *p;

 	Ctlr *ctlr;

 	uchar ier, lcr, mcr, msr;

 	ctlr = uart->regs;

-	p = malloc(READSTR);

 	mcr = ctlr->sticky[Mcr];

 	msr = csr8r(ctlr, Msr);

 	ier = ctlr->sticky[Ier];

 	lcr = ctlr->sticky[Lcr];

-	snprint(p, READSTR,

+	return seprint(p, e,

 		"b%d c%d d%d e%d l%d m%d p%c r%d s%d i%d\n"

 		"dev(%d) type(%d) framing(%d) overruns(%d) "

 		"berr(%d) serr(%d)%s%s%s%s\n",

@@ -214,10 +212,6 @@

 		(msr & Dcd) ? " dcd": "",

 		(msr & Ri) ? " ring": ""

 	);

-	n = readstr(offset, buf, n, p);

-	free(p);

-

-	return n;

 }

 static void

--- a/sys/src/9/pc/uartaxp.c

+++ b/sys/src/9/pc/uartaxp.c

@@ -254,21 +254,19 @@

 	}

 }

-static long

-axpstatus(Uart* uart, void* buf, long n, long offset)

+static char*

+axpstatus(Uart* uart, char *p, char *e)

 {

-	char *p;

 	Ccb *ccb;

 	u16int bs, fstat, ms;

 	ccb = ((Cc*)(uart->regs))->ccb;

-	p = smalloc(READSTR);

 	bs = ccb->bs;

 	fstat = ccb->df;

 	ms = ccb->ms;

-	snprint(p, READSTR,

+	return seprint(p, e,

 		"b%d c%d d%d e%d l%d m%d p%c r%d s%d i%d\n"

 		"dev(%d) type(%d) framing(%d) overruns(%d) "

 		"berr(%d) serr(%d)%s%s%s%s\n",

@@ -295,10 +293,6 @@

 		(ms & Sdcd) ? " dcd"  : "",

 		(ms & Sri) ? " ring" : ""

 	);

-	n = readstr(offset, buf, n, p);

-	free(p);

-

-	return n;

 }

 static void

--- a/sys/src/9/pc/uarti8250.c

+++ b/sys/src/9/pc/uarti8250.c

@@ -150,20 +150,18 @@

 #define csr8r(c, r)	inb((c)->io+(r))

 #define csr8w(c, r, v)	outb((c)->io+(r), (c)->sticky[(r)]|(v))

-static long

-i8250status(Uart* uart, void* buf, long n, long offset)

+static char*

+i8250status(Uart* uart, char *p, char *e)

 {

-	char *p;

 	Ctlr *ctlr;

 	uchar ier, lcr, mcr, msr;

 	ctlr = uart->regs;

-	p = smalloc(READSTR);

 	mcr = ctlr->sticky[Mcr];

 	msr = csr8r(ctlr, Msr);

 	ier = ctlr->sticky[Ier];

 	lcr = ctlr->sticky[Lcr];

-	snprint(p, READSTR,

+	return seprint(p, e,

 		"b%d c%d d%d e%d l%d m%d p%c r%d s%d i%d\n"

 		"dev(%d) type(%d) framing(%d) overruns(%d) "

 		"berr(%d) serr(%d)%s%s%s%s\n",

@@ -190,10 +188,6 @@

 		(msr & Dcd) ? " dcd": "",

 		(msr & Ri) ? " ring": ""

 	);

-	n = readstr(offset, buf, n, p);

-	free(p);

-

-	return n;

 }

 static void

--- a/sys/src/9/port/devuart.c

+++ b/sys/src/9/port/devuart.c

@@ -399,6 +399,7 @@

 uartread(Chan *c, void *buf, long n, vlong off)

 {

 	Uart *p;

+	char *s;

 	ulong offset = off;

 	if(c->qid.type & QTDIR){

@@ -413,7 +414,29 @@

 	case Nctlqid:

 		return readnum(offset, buf, n, NETID(c->qid.path), NUMSIZE);

 	case Nstatqid:

-		return (*p->phys->status)(p, buf, n, offset);

+		if(off >= READSTR)

+			return 0;

+		s = smalloc(READSTR);

+		if(waserror()){

+			free(s);

+			nexterror();

+		}

+		if(p->phys->status == nil)

+			seprint(s, s + READSTR,

+				"b%d\n"

+				"dev(%d) type(%d) framing(%d) overruns(%d) "

+				"berr(%d) serr(%d)\n",

+				p->baud,

+				p->dev, p->type, p->ferr, p->oerr,

+				p->berr, p->serr);

+		else {

+			s[0] = '\0';

+			(*p->phys->status)(p, s, s + READSTR);

+		}

+		n = readstr(offset, buf, n, s);

+		free(s);

+		poperror();

+		return n;

 	}

 	return 0;

--- a/sys/src/9/port/portdat.h

+++ b/sys/src/9/port/portdat.h

@@ -881,7 +881,7 @@

 	void	(*modemctl)(Uart*, int);

 	void	(*rts)(Uart*, int);

 	void	(*dtr)(Uart*, int);

-	long	(*status)(Uart*, void*, long, long);

+	char*	(*status)(Uart*, char*, char*);

 	void	(*fifo)(Uart*, int);

 	void	(*power)(Uart*, int);

 	int	(*getc)(Uart*);	/* polling versions, for iprint, rdb */

--- a/sys/src/9/ppc/uartsaturn.c

+++ b/sys/src/9/ppc/uartsaturn.c

@@ -132,14 +132,13 @@

 }

-static long

-sustatus(Uart* uart, void* buf, long n, long offset)

+static char*

+sustatus(Uart* uart, char *p, char *e)

 {

 	Saturnuart *su;

-	char p[128];

 	su = ((UartData*)uart->regs)->su;

-	snprint(p, sizeof p, "b%d c%d e%d l%d m0 p%c s%d i1\n"

+	return seprint(p, e, "b%d c%d e%d l%d m0 p%c s%d i1\n"

 		"dev(%d) type(%d) framing(%d) overruns(%d)\n",

 		uart->baud,

@@ -153,10 +152,6 @@

 		uart->type,

 		uart->ferr,

 		uart->oerr);

-	n = readstr(offset, buf, n, p);

-	free(p);

-

-	return n;

 }

 static void

--- a/sys/src/9/ppc/uartsmc.c

+++ b/sys/src/9/ppc/uartsmc.c

@@ -212,14 +212,13 @@

 	ud->enabled = 1;

 }

-static long

-smcstatus(Uart* uart, void* buf, long n, long offset)

+static char*

+smcstatus(Uart* uart, char *p, char *e)

 {

 	SMC *sp;

-	char p[128];

 	sp = ((UartData*)uart->regs)->smc;

-	snprint(p, sizeof p, "b%d c%d e%d l%d m0 p%c s%d i1\n"

+	return seprint(p, e, "b%d c%d e%d l%d m0 p%c s%d i1\n"

 		"dev(%d) type(%d) framing(%d) overruns(%d)\n",

 		uart->baud,

@@ -234,10 +233,6 @@

 		uart->ferr,

 		uart->oerr 

 	);

-	n = readstr(offset, buf, n, p);

-	free(p);

-

-	return n;

 }

 static void

--- a/sys/src/9/sgi/uartarcs.c

+++ b/sys/src/9/sgi/uartarcs.c

@@ -159,12 +159,6 @@

 	return 0;

 }

-static long

-status(Uart *, void *, long, long)

-{

-	return 0;

-}

-

 void

 uartarcsputc(Uart*, int c)

 {

@@ -192,7 +186,6 @@

 	.modemctl	= donothing,

 	.rts		= donothing,

 	.dtr		= donothing,

-	.status		= status,

 	.fifo		= donothing,

 	.getc		= uartarcsgetc,

--- a/sys/src/9/teg2/uarti8250.c

+++ b/sys/src/9/teg2/uarti8250.c

@@ -145,20 +145,18 @@

 #define csr8w(c, r, v)	((c)->io[r] = (c)->sticky[r] | (v), coherence())

 #define csr8o(c, r, v)	((c)->io[r] = (v), coherence())

-static long

-i8250status(Uart* uart, void* buf, long n, long offset)

+static char*

+i8250status(Uart* uart, char *p, char *e)

 {

-	char *p;

 	Ctlr *ctlr;

 	uchar ier, lcr, mcr, msr;

 	ctlr = uart->regs;

-	p = malloc(READSTR);

 	mcr = ctlr->sticky[Mcr];

 	msr = csr8r(ctlr, Msr);

 	ier = ctlr->sticky[Ier];

 	lcr = ctlr->sticky[Lcr];

-	snprint(p, READSTR,

+	return seprint(p, e,

 		"b%d c%d d%d e%d l%d m%d p%c r%d s%d i%d\n"

 		"dev(%d) type(%d) framing(%d) overruns(%d) "

 		"berr(%d) serr(%d)%s%s%s%s\n",

@@ -185,10 +183,6 @@

 		(msr & Dcd) ? " dcd": "",

 		(msr & Ri) ? " ring": ""

 	);

-	n = readstr(offset, buf, n, p);

-	free(p);

-

-	return n;

 }

 static void

--- a/sys/src/9/xen/uartxen.c

+++ b/sys/src/9/xen/uartxen.c

@@ -199,33 +199,6 @@

 	return 0;

 }

-static long

-status(Uart *uart, void *buf, long n, long offset)

-{

-	char *p;

-

-	p = malloc(READSTR);

-	if(p == nil)

-		error(Enomem);

-	snprint(p, READSTR,

-		"b%d\n"

-		"dev(%d) type(%d) framing(%d) overruns(%d) "

-		"berr(%d) serr(%d)\n",

-

-		uart->baud,

-		uart->dev,

-		uart->type,

-		uart->ferr,

-		uart->oerr,

-		uart->berr,

-		uart->serr

-	);

-	n = readstr(offset, buf, n, p);

-	free(p);

-

-	return n;

-}

-

 void

 xenputc(Uart*, int c)

 {

@@ -279,7 +252,6 @@

 	.modemctl	= donothing,

 	.rts		= donothing,

 	.dtr		= donothing,

-	.status		= status,

 	.fifo		= donothing,

 	.getc		= xengetc,

-- 

⑨