From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 122951 invoked by alias); 6 Dec 2016 14:03:20 -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 122917 invoked by uid 89); 6 Dec 2016 14:03:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.8 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=ted, Ted, concrete, sk:ac_repl X-HELO: mx1.redhat.com Subject: Re: [PATCH v9] Add getentropy, getrandom, [BZ #17252] To: Zack Weinberg , Torvald Riegel References: <5b8e5866-d071-9e2c-54e7-2ccf857a2fd8@redhat.com> <97f4db91-49d9-83fa-9f67-6cc718629160@redhat.com> <1480697250.14990.52.camel@redhat.com> <680d0bed-b164-b809-d672-e0278fe08d7e@redhat.com> <90908be7-c7db-46f2-a635-27dc5604e47f@panix.com> Cc: GNU C Library From: Florian Weimer Message-ID: <68cc12fb-e316-f727-8964-58561282744f@redhat.com> Date: Tue, 06 Dec 2016 14:03:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <90908be7-c7db-46f2-a635-27dc5604e47f@panix.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-SW-Source: 2016-12/txt/msg00163.txt.bz2 On 12/06/2016 02:41 PM, Zack Weinberg wrote: > On 12/06/2016 07:55 AM, Florian Weimer wrote: >> On 12/02/2016 05:47 PM, Torvald Riegel wrote: >>> On Wed, 2016-11-30 at 17:15 +0100, Florian Weimer wrote: >>>> On 11/30/2016 02:33 PM, Florian Weimer wrote: >>>>> This iteration of the patch implements both getrandom and getentropy at >>>>> the same time. >>> >>> This basically looks good to me (though I'm no expert on the actual >>> syscall etc.). >> >> Thanks. >> >> Zack, would you comment as well, please? > > The 256-byte limit is unfortunate but I see why we want it. > > I think you should remove this assertion: > > + /* The Linux implementation never returns zero if the length > + argument is not zero, and does not perform a short read for > + sizes <= 256. */ > + assert (bytes == length); > > it strikes me as Knowing Too Much about the kernel interface. Based on Ted Ts'o's comments, I think it's part of the userspace ABI, but we can still leave it out. We need a way to deal with a return value of 0 from the system call (another “can't happen” case, from which we can't really recover locally), and returning EIO is probably fine there. > My only other remaining concern is the name mangling, and unfortunately > we really do have to resolve that before this can be committed, because > we'll be stuck with whatever decision we make here forever. > > I still don't really understand what problems you are trying to solve by > mangling names, I still think that ad-hoc addition of mangled names with > forcible redirection in the headers is unlikely to be the *correct* fix > to whatever the problems actually are, and most importantly of all, I > still don't understand why you are convinced *these particular symbols* > need "interposition protection". They are special because interposition of a function which does something else will not always cause observable behavior differences. If an application redefines fread so that nothing is actually read from the stream, then this is pretty obvious. If getrandom or getentropy stop overwriting their argument buffers, then that's not going to cause immediate breakage. > You said > >> getentropy definitely needs interposition protection because it is >> frequently redefined. We'll need to rebuild a distribution to see if >> the current approach is sufficient. For consistency, I also added >> interposition protection for getrandom. > > and this makes absolutely no sense at all to me. Is it not the case > that people are defining getentropy and/or getrandom *because* libc > doesn't? No, there is code out there which checks for the availability of getrandom (in syscall form) and uses it to implement its own version of getentropy. > Won't their build systems notice (via AC_REPLACE_FUNCS or > equivalent) that libc is now defining them, and stop? I don't think so. This doesn't happen universally. > A concrete example of a real program or combination of programs, that > will break if we don't do this, would be really helpful to me. Not a > demo, please, something that already exists in the wild. Here's an example, I think: Thanks, Florian