From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14932 invoked by alias); 3 Dec 2013 23:01:36 -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 14911 invoked by uid 89); 3 Dec 2013 23:01:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=AWL,BAYES_50,RDNS_NONE,SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-ve0-f173.google.com Received: from Unknown (HELO mail-ve0-f173.google.com) (209.85.128.173) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Tue, 03 Dec 2013 23:01:31 +0000 Received: by mail-ve0-f173.google.com with SMTP id oz11so10930124veb.4 for ; Tue, 03 Dec 2013 15:01:23 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=DWlrGm51975/wXaf+ouEc30CscVwk0kK1jlcSSswkmc=; b=cIMf5hv8vwcgpGTMRyHigUZ+cZTI/gKMIg88IRORzZH1EyAWmElKhqU4+LNjFKIZyM AxeOaMTZai1QQS1L2APcpjhzw204ADOypOPm5BtDmMlSAk4+IoFoOqPGv/B+tKUyIgIj YkaUKgc/ExCmzL8TCNt6h8eZEuKFZShIVwv9H3OUy5wWzy8Q3kGCYW0uT15WZXLy/k2A gZxcNqbq0QRhJMA9zb9T8TyhifMVtHI7OqS1SZzjWohz26TM1zupIpG+lpnAIMy1aCdJ aWHg+TTFPl55jJUm3LRxHYgQD9yECsu6neRIuiH/+rYcVrA+HYpITkdVZZJk25jXZUvT 58LQ== X-Gm-Message-State: ALoCoQmotT4RdGviHFE43Lc63oVZPqEjVuzh7WT430jaxAy7qb89E8FQ020EVBh91d/3zoLXODgt0E0r3ZPP6EfgcT4+bgecCVfy0igGuL8YO5x/SF+28h97Kb1Xiz/iPVGb1msdgYyNU1DDf5KvXz+kpJtoBJCdMSwyAsf5cLQlzNsLDMMyY+VUF8FGHdq/VAb/j09wYoXb7qnsvpHijgvGsYeG3lwOhQ== MIME-Version: 1.0 X-Received: by 10.58.168.205 with SMTP id zy13mr7482834veb.19.1386111683664; Tue, 03 Dec 2013 15:01:23 -0800 (PST) Received: by 10.52.163.52 with HTTP; Tue, 3 Dec 2013 15:01:23 -0800 (PST) In-Reply-To: <529E3F10.6030607@redhat.com> References: <529E3F10.6030607@redhat.com> Date: Tue, 03 Dec 2013 23:01:00 -0000 Message-ID: Subject: Re: [PATCH] PR 16286: Reading python value as string beyond declared size From: Doug Evans To: Pedro Alves Cc: gdb-patches , Joel Brobecker , Sterling Augustine Content-Type: text/plain; charset=ISO-8859-1 X-IsSubscribed: yes X-SW-Source: 2013-12/txt/msg00109.txt.bz2 On Tue, Dec 3, 2013 at 12:29 PM, Pedro Alves wrote: > On 12/02/2013 11:14 PM, Doug Evans wrote: >> + if (*length > 0) >> + fetchlimit = UINT_MAX; > > Shouldn't this be: > > if (*length > 0) > fetchlimit = *length; > > ? That is, if the caller specified a limit, why do we do over it? read_string will take min (len, fetchlimit), and I saw no value in passing fetchlimit = *length. > Couldn't this new check be merge above where we compute > fetchlimit to begin with? With the comment there adjusted to > something like: > > + /* If have an explicit requested length, use that as fetchlimit. > + Otherwise, if we know the size of the array, we can use it as > + a limit on the number of characters to be fetched. */ I want to keep fetchlimit and *length separate at this point. They have different meanings/purposes. > BTW, it looks like the not_lval/lval_internalvar path can > blindly read beyond the value's contents buffer, if *length > is bigger than the value's contents buffer size: > > /* If the string lives in GDB's memory instead of the inferior's, > then we just need to copy it to BUFFER. Also, since such strings > are arrays with known size, FETCHLIMIT will hold the size of the > array. */ > if ((VALUE_LVAL (value) == not_lval > || VALUE_LVAL (value) == lval_internalvar) > && fetchlimit != UINT_MAX) > { > int i; > const gdb_byte *contents = value_contents (value); > > /* If a length is specified, use that. */ > if (*length >= 0) > i = *length; > ^^^^^^^^^^^^^ > else > /* Otherwise, look for a null character. */ > for (i = 0; i < fetchlimit; i++) > if (extract_unsigned_integer (contents + i * width, > width, byte_order) == 0) > break; > > /* I is now either a user-defined length, the number of non-null > characters, or FETCHLIMIT. */ > *length = i * width; > *buffer = xmalloc (*length); > memcpy (*buffer, contents, *length); > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ It didn't look right to me either, but I was leaving digging deeper for another pass.