From owner-freebsd-bugs@FreeBSD.ORG Fri Mar 7 13:14:01 2008 Return-Path: Delivered-To: freebsd-bugs@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 51C6E1065675; Fri, 7 Mar 2008 13:14:01 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from cyrus.watson.org (cyrus.watson.org [209.31.154.42]) by mx1.freebsd.org (Postfix) with ESMTP id 25FB48FC1F; Fri, 7 Mar 2008 13:14:01 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from fledge.watson.org (fledge.watson.org [209.31.154.41]) by cyrus.watson.org (Postfix) with ESMTP id 78C4D46B83; Fri, 7 Mar 2008 08:14:00 -0500 (EST) Date: Fri, 7 Mar 2008 13:14:00 +0000 (GMT) From: Robert Watson X-X-Sender: robert@fledge.watson.org To: Michael Plass In-Reply-To: <6DC910A8-BA9A-4681-9D07-B87DE7261038@plass-family.net> Message-ID: <20080307131317.O25342@fledge.watson.org> References: <20071227224807.40C8F1702A@shuttle.plass-family.net> <20080306095345.S12811@fledge.watson.org> <6DC910A8-BA9A-4681-9D07-B87DE7261038@plass-family.net> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-bugs@FreeBSD.org, FreeBSD-gnats-submit@FreeBSD.org Subject: Re: kern/119079: [patch] DDB input routine reads/writes beyond end of buffer X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 07 Mar 2008 13:14:01 -0000 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 > > >