From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 39329 invoked by alias); 19 Apr 2017 15:17:17 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 39254 invoked by uid 89); 19 Apr 2017 15:17:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=ham version=3.3.2 spammy=exposure X-HELO: mail-qt0-f180.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=Prq3P2cZX6x2IbD0wKlaxFBSakyAOGzwau2zHpuW/kA=; b=QZlBjVVGEU0HZsIaZ32BrF7JChv6g+0+O5OfADB76eBRIdgGtHHQh7301ZZdBYZY/V pqMHm679phMCnB3/c0aLIN/bUox4RMTb/S7k2HSqMrCqVec8AQw8T7x+tEmf6HGMwRMz z+kEYF6w19AxyEyvQ8w73S3i6nq7rfhoW2dzCbAfoNtJYWeupWqjK1x/7vh7E3pfYlcU vj6/Ln1GdniZlGroJS7ngfs5Dmg6HEj2MV/RIiw84Hslx7gxrPOn3A64E+Hjx7P7x3u1 cRvE/NxcJI7Drf+OejEkawAHj3Q1Q/3iPfwuTt/ngsQhJ/hniHJs3awh6HEeF+j/n8wd 6tQA== X-Gm-Message-State: AN3rC/5p2uC/LrHFFSkcmu3XXmq3FqiPoIrp2Uuw04KO2W3XgL9AfBUu 1JnnM/M28bf6njwLHuldmQ== X-Received: by 10.237.59.198 with SMTP id s6mr3132099qte.97.1492615034220; Wed, 19 Apr 2017 08:17:14 -0700 (PDT) Subject: Re: Maintaining libio To: Florian Weimer , libc-alpha@sourceware.org References: <1470418850-22175-1-git-send-email-adhemerval.zanella@linaro.org> <1470688986-8798-1-git-send-email-adhemerval.zanella@linaro.org> <9c55532d-701b-c35d-335c-19a98f1ec1b8@linaro.org> <0ffea98c-b9f1-bcb2-37a4-c4a2f4235d1b@redhat.com> From: Adhemerval Zanella Message-ID: Date: Wed, 19 Apr 2017 15:17:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <0ffea98c-b9f1-bcb2-37a4-c4a2f4235d1b@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-04/txt/msg00379.txt.bz2 On 19/04/2017 12:02, Florian Weimer wrote: > On 04/19/2017 04:48 PM, Adhemerval Zanella wrote: >> >> >> On 19/04/2017 06:17, Florian Weimer wrote: >>> On 08/08/2016 10:43 PM, Adhemerval Zanella wrote: >>>> @@ -239,14 +254,14 @@ _IO_str_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode) >>>> if (mode == 0 && (fp->_flags & _IO_TIED_PUT_GET)) >>>> mode = (fp->_flags & _IO_CURRENTLY_PUTTING ? _IOS_OUTPUT : _IOS_INPUT); >>>> + bool was_writing = (fp->_IO_write_ptr > fp->_IO_write_base >>>> + || _IO_in_put_mode (fp)); >>>> + if (was_writing) >>>> + _IO_str_switch_to_get_mode (fp); >>> >>> This patch breaks backwards compatibility with applications which call _IO_str_seekoff directly. This is an exported function and it was originally intended as a building block for building custom streams, so we cannot change what it does just to fit glibc's internal needs, based on how the function is called from within glibc. >>> >>> But if we apply this standard of backwards compatibility, we cannot make *any* changes to libio (including important security hardening) without copying most of the code first. We have no tests which check the extended API behavior, and the interface is very much under-documented, too. >>> >>> What should we do here? >> >> Right, so should we revert the patch, reopen both bugs and rework all libio >> in this case (which might span on multiple releases)? I know that we should >> aim for compatibility where applicable, but I think blindly aim for it even >> for bug/out of conformance cases adds more maintainer burden that actually >> fixes real cases usage. >> >> For this specific case, the code is clearly buggy when ran a different libc >> for a non-specific gnu extension (open_memstream). Should we still provide >> buggy compat interfaces in this case (as we are still aiming to provide)? > > Sorry, I wasn't clear. The buggy interface is open_memstream. Fixing that is completely fine, no compatibility symbol is required. But _IO_str_seekoff is a completely different, allegedly public interface, and the existing callers most expect some concrete behavior from it. > > A hypothetical example of the same scenario: posix_fallocate used to have a bug that it did not work on O_APPEND descriptors because pwrite64 ignored the offset for them, as required by POSIX. We could have fixed that by changing pwrite64 not to ignore the write offset, but that's of course bogus because the specified semantics for pwrite64 require different behavior. > > What I'm trying to say is that similar, but undocumented requirements might well exist for the _IO_str_seekoff function. Hence my original comment that we'd need to make a copy first, fix the copy, and adjust internal callers to use the copy, leaving the original implementation alone. > > Things get worse once we start changing struct definitions. Then we basically have to duplicate everything that depends on those structs. > > I think that's not a good use of our time because it is very unlikely that there are any applications left which use these interfaces. Instead, I suggest that we make explicit that the internal libio interfaces are unsupported (because they are unsupportable), and remove them from the ABI, so that people who have those old binaries get clear dynamic linker failures instead of corrupted data. This seems a more reasonable approach, I realised it was about _IO_str_seekoff exposure right after send the email (sorry about that noise).