From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 94795 invoked by alias); 17 Sep 2018 18:42:19 -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 94786 invoked by uid 89); 17 Sep 2018 18:42:19 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.6 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 17 Sep 2018 18:42:18 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id AC644117F08; Mon, 17 Sep 2018 14:42:16 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id f9TsSuA6ZZkS; Mon, 17 Sep 2018 14:42:16 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 15107117EF3; Mon, 17 Sep 2018 14:42:07 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 452DC873D9; Mon, 17 Sep 2018 11:42:05 -0700 (PDT) Date: Mon, 17 Sep 2018 18:42:00 -0000 From: Joel Brobecker To: Alan Hayward Cc: gdb-patches@sourceware.org, nd@arm.com Subject: Re: [PATCH] Aarch64 SVE: Fix stack smashing when calling functions Message-ID: <20180917184205.GG19172@adacore.com> References: <20180917144540.78906-1-alan.hayward@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180917144540.78906-1-alan.hayward@arm.com> User-Agent: Mutt/1.9.4 (2018-02-28) X-SW-Source: 2018-09/txt/msg00598.txt.bz2 Hi Alan, > Using "call" on a function that passes arguments via float registers can cause > gdb to overflow buffers. > Ensure enough memory is reserved to hold a full FP register. > > 2018-09-17 Alan Hayward > > * aarch64-tdep.c (pass_in_v): Use register size. > (aarch64_extract_return_value): Likewise. > (aarch64_store_return_value): Likewise. Do we have a testcase already that demonstrates the problem? Otherwise, it would be nice to add one. > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c > index 6993e9061e..516eb138dc 100644 > --- a/gdb/aarch64-tdep.c > +++ b/gdb/aarch64-tdep.c > @@ -1358,7 +1358,10 @@ pass_in_v (struct gdbarch *gdbarch, > if (info->nsrn < 8) > { > int regnum = AARCH64_V0_REGNUM + info->nsrn; > - gdb_byte reg[V_REGISTER_SIZE]; > + /* Enough space for a full vector register. */ > + gdb_byte reg[register_size (gdbarch, regnum)]; > + gdb_static_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM); > + gdb_assert (len <= sizeof (reg)); Could you explain the relationship between making the buffer large enough, which is the purpose of this patch, and the assertion that AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM? I don't see a problem with that assertion, but for archeology purposes, it is better to decorelate changes that are independent. It helps better document why we introduced changes. > > info->argnum++; > info->nsrn++; > @@ -1929,7 +1932,10 @@ aarch64_extract_return_value (struct type *type, struct regcache *regs, > for (int i = 0; i < elements; i++) > { > int regno = AARCH64_V0_REGNUM + i; > - bfd_byte buf[V_REGISTER_SIZE]; > + /* Enough space for a full vector register. */ > + gdb_byte buf[register_size (gdbarch, regno)]; > + gdb_static_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM); > + gdb_assert (len <= sizeof (buf)); > > if (aarch64_debug) > { > @@ -2039,7 +2045,10 @@ aarch64_store_return_value (struct type *type, struct regcache *regs, > for (int i = 0; i < elements; i++) > { > int regno = AARCH64_V0_REGNUM + i; > - bfd_byte tmpbuf[V_REGISTER_SIZE]; > + /* Enough space for a full vector register. */ > + gdb_byte tmpbuf[register_size (gdbarch, regno)]; > + gdb_static_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM); > + gdb_assert (len <= sizeof (tmpbuf)); > > if (aarch64_debug) > { > -- > 2.15.2 (Apple Git-101.1) Thank you! -- Joel