From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24822 invoked by alias); 20 Mar 2015 16:24:12 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 24806 invoked by uid 89); 20 Mar 2015 16:24:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mail-ie0-f201.google.com Received: from mail-ie0-f201.google.com (HELO mail-ie0-f201.google.com) (209.85.223.201) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 20 Mar 2015 16:24:09 +0000 Received: by iebtr6 with SMTP id tr6so6738782ieb.1 for ; Fri, 20 Mar 2015 09:24:07 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:mime-version:content-type :content-transfer-encoding:message-id:date:to:cc:subject:in-reply-to :references; bh=sWlw06EQaxl5qZirvcSKqTJNcnhKLjs2TcxVAfBl39w=; b=cvYURIWHIq7y7dTGagyKGA+TIccLINjpMTlFOMxabNleh9Dmp85zpAXT7cso11c0iK zKP4LMf4O/kBr9ZX1TCJUaotkGysJS5fPsUspPUnau7DkZRaxsjsckSNN+iUST4vkIHg cYwSYLBccIE4Jwm68FkNqgv7FvyjqLlBQaUH7gIqFcoML2DsMvwWz1jlj05ZvwTDhOvo hfbGQJct4jBF/i+3g5Pt83dYz7gaQpb50NL6WRVPc3E06Wr4iVgZjnDlC/xMKzFsStC5 55lg4ASVJiqn1Kc/7LlZagFHir/o2RgFf4RD+oyx4To0HsGBgvAA3kRc7hetrtZOlZta yjJQ== X-Gm-Message-State: ALoCoQm3Sp+BCh/wpWT7AYrVXJdn7y48waeNG+R2ivM9wmR6sMZ+8VcZlDej7byaFr1OgkXhb3gXKmhIkZYjPSn8XdNz1cdaaFpNIFmupfIgFC2lV9Ns/RvxOMeTFXXTGzeKPDIdLpHOE1t8L1oP9kdb5NabJl7FpjI61uzdiMLLfXTGUELIU30= X-Received: by 10.43.55.212 with SMTP id vz20mr1977931icb.16.1426868647405; Fri, 20 Mar 2015 09:24:07 -0700 (PDT) Received: from corpmail-nozzle1-2.hot.corp.google.com ([100.108.1.103]) by gmr-mx.google.com with ESMTPS id 3si346852yhe.0.2015.03.20.09.24.06 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 20 Mar 2015 09:24:07 -0700 (PDT) Received: from ruffy2.mtv.corp.google.com ([172.17.128.107]) by corpmail-nozzle1-2.hot.corp.google.com with ESMTP id 84tKrnh5.1; Fri, 20 Mar 2015 09:24:07 -0700 From: Doug Evans MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <21772.18854.130306.303139@ruffy2.mtv.corp.google.com> Date: Fri, 20 Mar 2015 16:24:00 -0000 To: Eli Zaretskii Cc: anton@samba.org, gdb-patches@sourceware.org Subject: Re: [PATCH] TUI: Fix buffer overflow in tui_expand_tabs In-Reply-To: <838uesw0xp.fsf@gnu.org> References: <20150317103009.538f2b3d@kryten> <838uesw0xp.fsf@gnu.org> X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00637.txt.bz2 Eli Zaretskii writes: > > Date: Thu, 19 Mar 2015 15:57:58 -0700 > > From: Doug Evans > > Cc: gdb-patches , Eli Zaretskii > > > > > +2015-03-17 Anton Blanchard > > > + > > > + * tui/tui-io.c (tui_expand_tabs): Zero col before reusing. > > > + > > > 2015-03-16 John Baldwin > > > > > > * fbsd-tdep.c (fbsd_make_corefile_notes): Fetch all target registers > > > diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c > > > index a8af9b6..02ae17d 100644 > > > --- a/gdb/tui/tui-io.c > > > +++ b/gdb/tui/tui-io.c > > > @@ -690,7 +690,7 @@ tui_expand_tabs (const char *string, int col) > > > ret = q = xmalloc (strlen (string) + n_adjust + 1); > > > > > > /* 2. Copy the original string while replacing TABs with spaces. */ > > > - for (s = string; s; ) > > > + for (col = 0, s = string; s; ) > > > { > > > char *s1 = strpbrk (s, "\t"); > > > if (s1) > > > > Hi. > > > > col needs to be reset to its original value on function entry, right? > > I suggest changing the code so that col is left unmodified, > > and use a new variable to track the advance of col in both loops. > > Sorry about the bug. Does the below look correct? > > --- gdb/tui/tui-io.c~ 2015-02-20 19:11:44.000000000 +0200 > +++ gdb/tui/tui-io.c 2015-03-20 10:01:11.289375000 +0200 > @@ -761,6 +761,7 @@ tui_expand_tabs (const char *string, int > int n_adjust; > const char *s; > char *ret, *q; > + int nc = col; > > /* 1. How many additional characters do we need? */ > for (n_adjust = 0, s = string; s; ) Nit. To my eyes the following would be more readable. - int n_adjust; + int nc, n_adjust; const char *s; char *ret, *q; /* 1. How many additional characters do we need? */ - for (n_adjust = 0, s = string; s; ) + for (nc = col, n_adjust = 0, s = string; s; ) > @@ -768,10 +769,10 @@ tui_expand_tabs (const char *string, int > s = strpbrk (s, "\t"); > if (s) > { > - col += (s - string) + n_adjust; > + nc += (s - string) + n_adjust; > /* Adjustment for the next tab stop, minus one for the TAB > we replace with spaces. */ > - n_adjust += 8 - (col % 8) - 1; > + n_adjust += 8 - (nc % 8) - 1; > s++; > } > } > @@ -780,7 +781,7 @@ tui_expand_tabs (const char *string, int > ret = q = xmalloc (strlen (string) + n_adjust + 1); > > /* 2. Copy the original string while replacing TABs with spaces. */ > - for (s = string; s; ) > + for (s = string, nc = col; s; ) > { > char *s1 = strpbrk (s, "\t"); > if (s1) > @@ -789,12 +790,12 @@ tui_expand_tabs (const char *string, int > { > strncpy (q, s, s1 - s); > q += s1 - s; > - col += s1 - s; > + nc += s1 - s; > } > do { > *q++ = ' '; > - col++; > - } while ((col % 8) != 0); > + nc++; > + } while ((nc % 8) != 0); > s1++; > } > else Ok with that change. Thanks!