Date: Fri, 7 Mar 2008 13:14:00 +0000 (GMT) From: Robert Watson <rwatson@FreeBSD.org> To: Michael Plass <mfp49_freebsd@plass-family.net> Cc: freebsd-bugs@FreeBSD.org, FreeBSD-gnats-submit@FreeBSD.org Subject: Re: kern/119079: [patch] DDB input routine reads/writes beyond end of buffer Message-ID: <20080307131317.O25342@fledge.watson.org> In-Reply-To: <6DC910A8-BA9A-4681-9D07-B87DE7261038@plass-family.net> References: <20071227224807.40C8F1702A@shuttle.plass-family.net> <20080306095345.S12811@fledge.watson.org> <6DC910A8-BA9A-4681-9D07-B87DE7261038@plass-family.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 6 Mar 2008, Michael Plass wrote: > I was able to reproduce the problem in a version rigged to run in userspace. > Check what happens when you just type enough to fill the buffer (or until it > beeps) and then hit return. The normal chars are handled in the default > case of the big switch in db_inputchar(), and when the buffer is full (db_le > == db_lbuf_end), db_inputchar() returns 0. The driving loop calls again, > this time with the newline, and that gets stored this way: > case '\n': > /* FALLTHROUGH */ > case '\r': > *db_le++ = c; > return (1); leaving the newline in db_lbuf_end[0] and db_le == > db_lbuf_end + 1, so that the terminating nul is stored at db_lbuf_end[1]. > > I've only lightly tested it, but upon reexamination I think better fix is to > leave lsize alone in do_readline() and initialize > db_lbuf_end = lstart + lsize - 2; this will assure that each of the lines > in the history buffer have proper termination and storing the nul in the > initialization won't be needed. Ah, indeed. I like the revised fix better also. I've gone ahead and committed the revised patch and will MFC it in a few days. Thanks for the report/fixes! Robert N M Watson Computer Laboratory University of Cambridge > > The patch against 1.38 then looks like this: > --- db_input.c.orig 2008-03-06 22:09:04.000000000 -0800 > +++ db_input.c 2008-03-06 22:25:24.000000000 -0800 > @@ -302,6 +302,8 @@ > char * lstart; > int lsize; > { > + if (lsize < 2) > + return(0); > if (lsize != db_lhistlsize) { > /* > * (Re)initialize input line history. Throw away any > @@ -316,7 +318,7 @@ > db_force_whitespace(); /* synch output position */ > > db_lbuf_start = lstart; > - db_lbuf_end = lstart + lsize; > + db_lbuf_end = lstart + lsize - 2; > db_lc = lstart; > db_le = lstart; > > > Thanks for the care you've taken in committing this. > - Michael > > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080307131317.O25342>