From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 356883858296 for ; Wed, 6 Mar 2024 11:17:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 356883858296 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 356883858296 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709723839; cv=none; b=wKjHh81cFv9WZVngG9AXyvoqDPeG5z51T2U3vIo9riY9+yukZRQIgS7mpbM0jcLEWYWQnNIiqYO5AJZ3H5gEgGVpPAY18avgYxDnCifZORhT9AI8Tj8vyE4kdiqkCcN8dOzUpUuzjaDrNsE9cWCzK1KjXnPCmcaI2RkXX5EjusU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709723839; c=relaxed/simple; bh=rpKPyZzslIqvQWPxaKgkJS0SLZ5vcCGvdPl9NAkMULs=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=J83N6GQicHkhdrOqTHymEk6D1g6hP6vIv1ULKQMB79K7adnYNGA9O5U3GvKVn7M5uSg6owyVPauxNdYGNNALObP6RHgT3nEpPjm81uyHVO98CeDrQzwCaa9jv2eJ5tKPEiehPqLPmkvZLSi6vbEpHooP5DMRtEkQYI4Sv5P1NRA= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1709723834; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=NvQemnzI5kjyQP1vVSiY5gU+1AgzhknOxXGgQSI838E=; b=EvOPLmfPGrqJxGYCt3wKDGKxn6qPeXEuZ4KNegy5ZUlGvVaLyE8uDLddpK2ZglRlycfDnU H9OSpEuSD0wRU69y8WI0bvO2eGyc/3/IpwvQZMjZl6avbsaFdu75Enx1E5VpPUFAbWCPZA MRw+xOaofU2CXFQUHbzsp5HOC1bwhu0= Received: from mail-ej1-f72.google.com (mail-ej1-f72.google.com [209.85.218.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-504-fnnNbGTjNy66Zp_4X7k0-g-1; Wed, 06 Mar 2024 06:17:13 -0500 X-MC-Unique: fnnNbGTjNy66Zp_4X7k0-g-1 Received: by mail-ej1-f72.google.com with SMTP id a640c23a62f3a-a444b9dd222so418210466b.1 for ; Wed, 06 Mar 2024 03:17:13 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709723831; x=1710328631; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=NvQemnzI5kjyQP1vVSiY5gU+1AgzhknOxXGgQSI838E=; b=FitZ1GGYpTJ5VxtQQcqlFzxpkuzX3PEmmC6g2SzPpcNXvG1zEW17B2XhpzIgDBliq3 nWFHBqBovBakPizoHWgxOhilDMHufwKJIUgd96feUSHsuZXggRv8W5im1WJSZnnqyWbi ry9QlOtiCsXrSd1RSwIpY+ieaafX/pACNjl/7JVrZb5HTXfgt8Trnua7mz/ykmIeWjA/ FUsjzRhWC0JL8roiewSpMfsGvheABPb1IMxJWtXvOkpIbklEiOJdE2FoXfwkFGj7VhUN j1J68ivNrs1xO/LyopD1HTBBsK2REX8qb5JKL/nK6gVDXGtVL3cjGRadfNXhYVu2cEJm 3GNQ== X-Gm-Message-State: AOJu0Yy57NV8jAnSMauBA/p6peHvCz6Ab8dQD3aTMdP7lIkxiGsFybKW bqsGfTQTTSnvPpPaZbko+NGifzi7IWonyhUmCdw028vwn8rEcDsREI4pwiclKpId1dDcmDRGIW5 APisOIhhufvww7ssK55J+3Qr24ejqN6s3YD+i6TQBF7CJ27kT+wpVyu5CDBJOPAc= X-Received: by 2002:a17:906:13da:b0:a45:ab9b:4a28 with SMTP id g26-20020a17090613da00b00a45ab9b4a28mr2817044ejc.60.1709723830847; Wed, 06 Mar 2024 03:17:10 -0800 (PST) X-Google-Smtp-Source: AGHT+IGr29AQamTtHji/e3HjNoUgVsLZK/zkUvhQJpvASk0s1bAeTD6pkbrT7VEAojHBU7n9knHrwA== X-Received: by 2002:a17:906:13da:b0:a45:ab9b:4a28 with SMTP id g26-20020a17090613da00b00a45ab9b4a28mr2817030ejc.60.1709723830269; Wed, 06 Mar 2024 03:17:10 -0800 (PST) Received: from localhost (185.223.159.143.dyn.plus.net. [143.159.223.185]) by smtp.gmail.com with ESMTPSA id oy29-20020a170907105d00b00a43fe57b2basm7011297ejb.61.2024.03.06.03.17.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Mar 2024 03:17:09 -0800 (PST) From: Andrew Burgess To: Jan Beulich Cc: binutils@sourceware.org, Alan Modra Subject: Re: [PATCH] bfd: make _bfd_section_size_insane part of the public API In-Reply-To: References: <0c54069e-d907-4f03-8d7f-15374d4bfd6a@suse.com> <87frz58n7j.fsf@redhat.com> <877ckh8fjz.fsf@redhat.com> <8803d3e2-688e-43ac-b710-5237fab8b054@suse.com> <874jfl88jg.fsf@redhat.com> Date: Wed, 06 Mar 2024 11:17:09 +0000 Message-ID: <874jdjboei.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-6.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Jan Beulich writes: > On 10.01.2024 17:20, Andrew Burgess wrote: >> We're talking about the same _bfd_section_size_insane in bfd/section.c, >> right? >> >> I don't see any arbitrary limit in there. If there is such a limit then >> it's buried within all the very non-arbitrary sanity checks. >> >> This function isn't answering: "is this section larger than X", it is >> instead answering: "can this section possibly be read from this file". > > Hmm, first of all I have to apologize for having gone from memory, having > looked at the function only when I first saw your patch. Yet then what > you're talking about is merely the final check of the function, which I'd > view as fine to externalize. The whole rest of the function still seems > pretty (but not exclusively) heuristic to me. > > Anyway, Alan has indicated approval of making the function external, even > as-is. If I was to make a suggestion beyond the possible renaming / > inverting of sense, that would be to split the function into an "is valid" > part and an "is sane" one, with the latter kept internal. Hi Jan, This patch dropped from my radar for a while, mostly because I didn't really understand your last feedback -- I wanted to take some time then revisit this patch with fresh eyes -- then I completely forgot about this. You talk about splitting the function into two parts, an "is valid" part and an "is sane" part. I've looked at _bfd_section_size_insane again, and I still don't understand how you imagine the function being split. Here's a high level description of the function as I see it: 1. Get section size, 2. If section is one that we "know" will always be sane (i.e. was created within a tool rather than read from a BFD) then this is a sane section, 3. Get the file size, 4. If the section is a compressed section then, do some check that I don't really understand (despite the comment) against the section size. Later checks are done against the section's compressed size instead of the reported section size, 5. If the section is defined as outside the bounds of the file, or the section size is larger than the file size then there must be something wrong with the section, and the section is declared "insane", 6. Assume the section is sane. There's a small unknown in step #4 that I don't understand, but otherwise, this all seems pretty straight forward. I don't really understand how you imagine these parts being split. And if they were split, I don't understand why a user like GDB can't ask these questions about a section (or rather, we can ask the same question, all the functions/data that _bfd_section_size_insane uses are public, so GDB can just copy & paste this code, but I'd rather not do that). My motivation behind wanting to make this public is to allow GDB to perform this sanity check before trying to allocate memory, which I think will allow GDB to improve it's error handling when presented with an invalid BFD (ELF) object. Given Alan's feedback, I'm really just looking to see if you still have objections/requests for this patch, or if you're happy for this to be merged. Thanks, Andrew