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>