From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30886 invoked by alias); 31 Jan 2013 14:23:17 -0000 Received: (qmail 30875 invoked by uid 22791); 31 Jan 2013 14:23:15 -0000 X-SWARE-Spam-Status: No, hits=-2.6 required=5.0 tests=AWL,BAYES_00,KHOP_SPAMHAUS_DROP,KHOP_THREADED,RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from na3sys009aog108.obsmtp.com (HELO na3sys009aog108.obsmtp.com) (74.125.149.199) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 31 Jan 2013 14:23:10 +0000 Received: from mx10.qnx.com ([209.226.137.110]) (using TLSv1) by na3sys009aob108.postini.com ([74.125.148.12]) with SMTP ID DSNKUQp+TUAXRyj1TAjxjc304Cq2TjlUt6Ly@postini.com; Thu, 31 Jan 2013 06:23:10 PST Received: by mx10.qnx.com (Postfix, from userid 500) id 4386C20E05; Thu, 31 Jan 2013 09:23:09 -0500 (EST) Received: from exhts.ott.qnx.com (exch2 [10.222.2.136]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by mx10.qnx.com (Postfix) with ESMTPS id CE48E20ACE; Thu, 31 Jan 2013 09:23:08 -0500 (EST) Received: from [10.222.96.215] (10.222.2.5) by exch2.ott.qnx.com (10.222.2.136) with Microsoft SMTP Server id 14.2.318.4; Thu, 31 Jan 2013 09:23:08 -0500 Message-ID: <510A7E4B.4040608@qnx.com> Date: Thu, 31 Jan 2013 14:23:00 -0000 From: Aleksandar Ristovski User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130106 Thunderbird/17.0.2 MIME-Version: 1.0 Newsgroups: gmane.comp.gdb.patches To: Jan Kratochvil CC: "gdb-patches@sourceware.org" Subject: Re: [patch] validate binary before use References: <50D4C49A.6040502@qnx.com> <50D8B37A.20001@qnx.com> <20121225073709.GA11349@host2.jankratochvil.net> <50DCAA5C.3000301@qnx.com> <20121227205924.GA5109@host2.jankratochvil.net> <50DCB787.6020601@qnx.com> <20121227211328.GA5739@host2.jankratochvil.net> <50DCBBD1.7000707@qnx.com> <5107F591.304@qnx.com> <20130130191646.GA1034@host2.jankratochvil.net> In-Reply-To: <20130130191646.GA1034@host2.jankratochvil.net> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit 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 X-SW-Source: 2013-01/txt/msg00747.txt.bz2 Thanks for the review. Please see some of my comments inlined. On 13-01-30 02:16 PM, Jan Kratochvil wrote: > On Tue, 29 Jan 2013 17:15:13 +0100, Aleksandar Ristovski wrote: >> + /* Section vma is unrelocated. If SO_BASE_ADDR is zero, then >> + use ASECT->VMA as-is. If not, then use offset + base addr. */ >> + res = target_verify_memory (data, (so_base_addr > 0)? > > I do not see why to use target_verify_memory in this case. While your comment below is correct, I find, since we introduced target_verify_memory already, this to be "more correct". Well, it is equivalent to what you are suggesting and I was considering doing simply read_memory/memcmp here, but then figured, target_verify_memory is more semantically correct. > > target_verify_memory is there for large sections to compare only their 32-bit > checksum. But build-id is already only 20 bytes long, with the protocol > overhead the 4 vs. 20 bytes do not make a difference. And it needlessly > weakens the check, it also does some patching of target_verify_memory. > Just use target_read_memory and memcmp. This is all true; however my opinion is that fallback in target_verify_memory is correct implementation as it allows using target_verify_memory where semantically suitable (like this place IMO) regardless of whether actual target implements it or not (e.g. core doesn't need to implement it; if it did, the implementation would probably be exactly the same as the fallback). > > >> + so_base_addr + sect_vma_offset >> + : asect->vma, >> + size); > > so->abfd is not properly relocated (not sure why but it is so) but you can I find that l_addr_inferior remains to be 0 if prelinked object was not relocated. I haven't looked at gnu ld, but I would expect it's missing to set it correctly (this is misfortunate). > iterate so->sections..so->sections_end which contains relocated ADDR (=target > VMA). Then you can drop the svr4_unrelocated_vma and other calculations > around. Ok. I think this is what I tried first but then some testcases would fail. Will revisit. I will be posting new patch with other comments addressed, and please provide feedback on the above. Thanks, Aleksandar