From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 69304 invoked by alias); 10 Dec 2019 16:26:01 -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 64955 invoked by uid 89); 10 Dec 2019 16:25:58 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-5.8 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.1 spammy=honored, harness, inclined, IMHO X-HELO: us-smtp-1.mimecast.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1575995154; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=AINv26p2hc40saxKbsTexY5rxnSqvmm7PtM52a/8utM=; b=B0fJD4DhfcCNmLbl5jYaFQgpGY8Oy2EMtm3driKTVkE+30daBApb66NiF5n0x9y0JUQhFI baxq/gjzwTHstiPHd9Sxq3yuVqx4oPVlqOLjAVDAMBX4BZvFwg/8vuVWPBPk6HnMHPfRRk Y1si9IEP/4sY49jqAHLaEEmHufbbKzY= Return-Path: Subject: Re: RFC: tunables failure indications... To: Siddhesh Poyarekar , DJ Delorie , libc-alpha@sourceware.org References: From: Carlos O'Donell Message-ID: <4623b04a-d786-f10a-6681-e00b00b6bb5a@redhat.com> Date: Tue, 10 Dec 2019 16:26:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-12/txt/msg00322.txt.bz2 On 12/6/19 3:32 AM, Siddhesh Poyarekar wrote: > On 06/12/19 4:14 am, DJ Delorie wrote: >> Re: https://sourceware.org/bugzilla/show_bug.cgi?id=25035 >> "sbrk() failure handled poorly in tunables_strdup" > > As I understand it, there seem to be a few different problems in that bug: > > 1. The env gets updated even when strdup returns NULL, which is a bug > 2. Tunables may get ignored on internal errors > 3. Tunables may may get on invalid input > 4. brk failed > >> The current failure mode for tunables is: if we can't allocate memory, >> we delete the tunables env var and pretend it didn't exist, without >> telling the user anything happened. > > That was a deliberate design decision back then because tunables were > considered non-critical, i.e. they're not guaranteed to work in all > situations and as such, shouldn't hamper core functionality just because > it couldn't work. So problem (2) and (3) are by design. > >> I think we should do better, but I don't know what. >> >> IMHO tunables are "hints" but there should be *some* feedback when a >> tunable can't be honored because of internal failures (vs tunables >> that don't apply to the current arch, etc). >> >> In the BZ noted above, where *sbrk* fails, I'm inclined to just >> SIGSEGV because... well... *sbrk* just failed at the start of a >> program. Something has gone horribly wrong, even if the program >> continues to run normally after that (it happens in our own testsuite >> sometimes). > > That's a fair point. I would agree with a patch that fails > catastrophically on a failed brk or similar internal errors as opposed > to invalid values from users. That is, we make (2) fail with an error exit. I agree. Not having enough memory to carry out operations like reading the tunables should be a critical failure. The tunables are environment variables so they will naturally be limited to the size of the usuable environment, and so this is all normally expected behaviour. >> The helper functions in malloc return error codes for invalid values, >> which the tunables harness completely ignores. Not even a printf. > > This replicates what we used to do earlier when setting the > MALLOC_MMAP_MAX_, etc environment variables and it follows the same > principle that it shouldn't hamper core functionality. Maybe there's a > valid use case to have debug messages (when built with a flag set for > example) that get explicitly enabled to dump warnings on invalid tunable > values. Tunables are inherently functionality that the loader is brining online during process startup. It may be sensible to add a LD_DEBUG=tunables to that list and print more verbose tunables processing information? > As for (4), that's super-odd even with randomization. Is it possible > that somehow brk with ASLR enabled fails when the new brk end is not > page aligned? No, in the case DJ and I were looking at this was a static vs. ASLR vs. kernel VMA layout issue where we still have kernel issues on less maintstream architectures. We still need to fail safe in those cases and I think we should just shut the process down with appropriate error messages. -- Cheers, Carlos.