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.129.124]) by sourceware.org (Postfix) with ESMTPS id 889053858D38 for ; Wed, 10 Jan 2024 11:03:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 889053858D38 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 889053858D38 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704884614; cv=none; b=PeihurkbYMwVgl+syuVs4Yv5Wcg4Ei3nMNkD75wqG7lVuyFXyy3fbHFUNWkGE4tad60mzE7+ofZfg7ID5mO+rf1O84oHyRE9MDYO8EqY5tgdvScH3NYQ32ZWnqgRmxMwszkDfAVDtO0zHkEMckL+nv0JE9YWZ5ZGKUUylUVFvNY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704884614; c=relaxed/simple; bh=riWIt2ONpjbvXPyqqy4lYf06tS9QmdPz45EjHewSd88=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=Lx8ybcGXDDbk9zKVK31URGUyoJjtsiN4jCzUxmEZuH+Qn1PA0uYrl+/wq3Y6sdNDweY69mACFsJiebtzm1Kt7Y4HrdLXBtZAtM1/HLnSfIwos7lNadOxGjxlhJqhkysOO0oeCD0zpWYUV5lR/+A6JQAe0cVFs1HysBv3ipS8vnE= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704884612; 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=NKj1P67sIgFpyEfUGRxD6DIORuZvRYiBRHvKItge/aU=; b=D6nYLl7rR29Ag1SX6fHltp+FAcaI1mgbSM8my4W7Utgy/t0ENG9PwsxY4ghRocjYsofH+0 AvDgh8cF7yC9SbM0wVGx8ytN/90mSoieS5nm+tOIhiFRYnUQu04baHEntPr0M+x+cOCRk5 N2+8Hdh5BkRsT/dw26p2ALJ604ED3k4= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-557-riacDTJvNB-syfZ7EGBUHg-1; Wed, 10 Jan 2024 06:03:31 -0500 X-MC-Unique: riacDTJvNB-syfZ7EGBUHg-1 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-40e46bceed8so23274595e9.0 for ; Wed, 10 Jan 2024 03:03:30 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704884609; x=1705489409; 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=NKj1P67sIgFpyEfUGRxD6DIORuZvRYiBRHvKItge/aU=; b=m9EY395uLdX9AEoCzOrHC1/9tbR8QimCPcz0UuetBY1grZQZo6yak3uDgv/N6xBA8z s/r8PZ88pew0R0TWAy+vZkUX9/frklBTHTD2gpgCXAf7VTrfnExV5SdRIi2U9Wx6u0zm K1k3n6hFZUXgBpeQZiJWDBVJgtJ7cyUBr3FpG5ziUzLUytVHoxY0pTyxke1Aog3oju8V MhEbAEsLzxN3ex4EaVN9vnnO0Zp2t2PLn+3cItvIU7Cty4WOZ0w1TqPbtbS+1kMNo2J5 22uvYworrPeeH0p0oL5mb6iPW+c26T5pCkhEF8NUiukTODnacQTFJcWL+xBgh2DcjDZL hfnA== X-Gm-Message-State: AOJu0Yxij1jYGcenAgV0V2Yc4rccmYCY8BCEJbqfTu7w8wSBngEUyGeQ JzmvFwDOgnvVGoNwVMvkDMDQDtAXnFm4Wt+gD9uaCW43bhVJlNVh4nijxoIh2/elmSFrcUPR8Ru XsGE4S+0d3XfvKgXXHrQdT/PXzwi97Hos1A== X-Received: by 2002:a7b:c3d9:0:b0:40e:4df2:5d17 with SMTP id t25-20020a7bc3d9000000b0040e4df25d17mr498113wmj.65.1704884609366; Wed, 10 Jan 2024 03:03:29 -0800 (PST) X-Google-Smtp-Source: AGHT+IHO1UUTAoVhIfRiFW1mCSwE5u5Q6SXd42R46WTL3v6iFXTu/419DXy0Tb5XJIoOOQ8TxGMWxQ== X-Received: by 2002:a7b:c3d9:0:b0:40e:4df2:5d17 with SMTP id t25-20020a7bc3d9000000b0040e4df25d17mr498109wmj.65.1704884609043; Wed, 10 Jan 2024 03:03:29 -0800 (PST) Received: from localhost (185.223.159.143.dyn.plus.net. [143.159.223.185]) by smtp.gmail.com with ESMTPSA id n15-20020a05600c3b8f00b0040d5a5c523csm1805844wms.1.2024.01.10.03.03.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 10 Jan 2024 03:03:28 -0800 (PST) From: Andrew Burgess To: Jan Beulich Cc: binutils@sourceware.org Subject: Re: [PATCH] bfd: make _bfd_section_size_insane part of the public API In-Reply-To: <0c54069e-d907-4f03-8d7f-15374d4bfd6a@suse.com> References: <0c54069e-d907-4f03-8d7f-15374d4bfd6a@suse.com> Date: Wed, 10 Jan 2024 11:03:28 +0000 Message-ID: <87frz58n7j.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=-7.1 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 06.12.2023 17:15, Andrew Burgess wrote: >> If a BFD user is making use of a function like >> bfd_get_section_contents to read a section into a pre-allocated >> buffer, then that BFD user might also want to make use of >> _bfd_section_size_insane prior to allocating the buffer they intend to >> use in order to validate that the buffer size that plan to allocate is >> sane. >> >> This commit makes _bfd_section_size_insane public, by renaming it to >> bfd_section_size_insane. >> >> I've updated the existing uses within bfd/, I don't believe this >> function is used outside of bfd/ currently. >> >> One place that I plan to make use of this function is in >> gdb/gdb_bfd.c, in the function gdb_bfd_get_full_section_contents. >> This change isn't included in this commit, but will come later if/when >> this has been merged into bfd. > > Having seen your ping (and no other response), let me share my view: > This function implements a certain policy, internal to the library. > By exposing it, you would make external users dependent upon this > specific policy. What if later we change our view on what's "insane"? I would expect and want external users to get the updated definition. The function name of "insane" is a little unfortunate. I think if the function had a better name then this change would seem far less contentious. Consider a name of: validate_section_size_against_other_bfd_infernal_properties_of_the_elf_to_ensure_that_the_requested_size_is_likely_valid() > IOW external consumers want to implement their own, independent policy > (if so desired). Sure, consumers _could_ implement their own policy, but IMHO, this would be far worse than exposing the *_insane() function. What I (as a consumer) want is to check if the size that the BFD library is reporting is valid or not. To do that I need to check details of the ELF that I, as a BFD users, shouldn't have to bother with. (I thought) the point of BFD was to abstract details of the file format. > Taking your intended usage example, things would be different if e.g. > bfd_get_full_section_contents() itself used this check unconditionally. > Then I could see a desire to have a way of checking up front whether > allocating a buffer makes sense at all. And really I consider it > questionable for bfd_get_full_section_contents(), when asked to > allocate a buffer, to actually enforce such a library-internal policy. > Like with exposing bfd_section_size_insane(), any change to the policy > may affect external users in unexpected ways. I don't understand this paragraph at all. I'm sure I must be reading it wrong, but it feels like you're saying we shouldn't use bfd_section_size_insane(), which would mean we don't check for this one particular error case, but I'm not sure why you'd feel that way. Like I said, I'm sure that's _not_ what you're suggesting, I just don't see what it is you are trying to say. You start this paragraph by saying "Taking your intended usage example, ..." but don't really offer an alternative solution. I'd be interested if you did have some thoughts. Maybe a better solution is to change bfd_get_section_size() so that this function doesn't always just return the recorded section size, but instead returns 0 (or maybe -1 to indicate an error?) based on calling bfd_section_size_insane()? This feels far more risky as there's likely many calls to bfd_section_size() in the wild that don't expect to get back a size of 0.... but maybe that's a cleaner solution? Or maybe we just need to rewrite this corner of GDB to avoid having GDB allocate the buffers :/ Seems like an unfortunate conclusion... Anyway, thanks for your thoughts, Andrew