From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oo1-xc31.google.com (mail-oo1-xc31.google.com [IPv6:2607:f8b0:4864:20::c31]) by sourceware.org (Postfix) with ESMTPS id 9251D38618E3 for ; Wed, 4 Oct 2023 16:57:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9251D38618E3 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-oo1-xc31.google.com with SMTP id 006d021491bc7-57eaaba78d0so15130eaf.0 for ; Wed, 04 Oct 2023 09:57:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1696438666; x=1697043466; darn=sourceware.org; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=CC5Ii1uvR2uoCq7tFzgaNubMEI9JD46ROeSFJ87DwDA=; b=NOzc+AHZv5C4LNVYXIHaLqwjUmZO1io397H7yJYzx6IvveM2ELUHUYAeRe8zEEoghO D3RyqXoDq4U0KOvzsimiFmnz90mkLPl/cprhS8bBotnjFGBwLQQC1M6VKFqcs0mIg/H8 WUCYQMgK3oP+TGjYBS2CKMFuB/VOim/aWLanRT0QZaSr2n5/ubuwhX9IM1t8Aq+cVCTy VWRG0LZCGscGbNtQ4RrPt0F7+Iq75OsoWV1JG65QTlmOwabvnNSxevg8km/OflQbyZfo wr2ZEHSgl+uYqbzsC73A3vdFrNdF3HH1bQfq3GRW+Pt8RozbxdOq8Rsd1RTkqJwPwGSB 5pCw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696438666; x=1697043466; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=CC5Ii1uvR2uoCq7tFzgaNubMEI9JD46ROeSFJ87DwDA=; b=GshORlyLpX0quZLTQ9ItGTwpiDc9adAyZz1+3fCHJgkm9EVEXBpveG5c9ORV6Xpkbd DikLF4O97Rp5D1eXcKrH5i6i4kHY6wyUYyp+qc8IveUcnBtbgMQ2P4d2MyOPg9oNfHXb H7lVK/ugFwhEeCo2JSb/Mpkh7tbCxnh8G/ciBPnlGPL4fxAdW0WiIZjML4BSpdC6u+Oy x9Q32OSLzAC12bNQcoAgmF3tBFWpAv3uukLMNqoL10I61YmX3H0VCk7Zj/nGtAO+Fca2 Slt9soyEso3WeGY7t+Hd3CctFagTsdlmaDxlFtS2GKCpSBusllPNfqlsC+nRfsBGmJU0 kcCg== X-Gm-Message-State: AOJu0Yy4mcZHu1YklsTVwKwDB81t72bmNSltOi2dSx3uSYe2vvFHP7Fc zw/Z3Nxg5sQOlsjjCkStV97hD90yBCc5b8BSTYxhGg== X-Google-Smtp-Source: AGHT+IGMK/bBSrE6r0DezuVb4nvdD9fKKb8EUXHT4Bk/WXDerxy5XxPVvZX3M74r2yZM+Q6B9MyOYA== X-Received: by 2002:a05:6358:918b:b0:14f:3874:2746 with SMTP id j11-20020a056358918b00b0014f38742746mr2821677rwa.23.1696438665726; Wed, 04 Oct 2023 09:57:45 -0700 (PDT) Received: from ?IPV6:2804:1b3:a7c1:feaf:b959:23ff:3a18:c5dc? ([2804:1b3:a7c1:feaf:b959:23ff:3a18:c5dc]) by smtp.gmail.com with ESMTPSA id f17-20020a17090a4a9100b0026b6d0a68c5sm1818591pjh.18.2023.10.04.09.57.44 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 04 Oct 2023 09:57:45 -0700 (PDT) Message-ID: <3484e51b-af95-4f11-8d15-5ec21c0827c0@linaro.org> Date: Wed, 4 Oct 2023 13:57:42 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3] Fix FORTIFY_SOURCE false positive Content-Language: en-US To: =?UTF-8?Q?Volker_Wei=C3=9Fmann?= , Siddhesh Poyarekar , libc-alpha@sourceware.org References: <20231003171844.9586-1-volker.weissmann@gmx.de> <74fc573f-e83e-4383-af94-95f49d9ea1b2@gmx.de> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: <74fc573f-e83e-4383-af94-95f49d9ea1b2@gmx.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP 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: On 04/10/23 11:43, Volker Weißmann wrote: > I thought about my patch again... > > If an attacker can make the victim-program leak file descriptors, this > can be used to defeat string fortification. > > Since leaking file-descriptors is normally not that bad (normally, it > cannot lead to anything worse than a DOS), programmers/security auditors > might be less careful in ensuring that no fd leaks. > > It doesn't even have to be a true leak, image if e.g. the attacker > controls python code that runs inside an interpreter that does some > sandboxing. Then the attacker could do something like: > > with open("/dev/zero") as file1: >     with open("/dev/zero") as file2: >     ... >         with open("/dev/zero") as file1023: >             trigger_formatstring_bug_in_the_python_interpreter() > > to break out of the sandbox. > > I know I'm being a bit paranoid, but glibc is used *everywhere*. > > I think instead of "return 1;" we should do > > __libc_fatal ("*** too many open file descriptors ***\n"); > > instead. The same if also check for procfs support, meaning that if it is not accessible the process will start to abort execution. Not sure about what kind of breakage it would incur, but it should reasonable to abort on both cases since this is done iff fortify is enabled. > > > On 03.10.23 19:25, Siddhesh Poyarekar wrote: >> On 2023-10-03 13:18, Volker Weißmann wrote: >>> When -D_FORTIFY_SOURCE=2 was given during compilation, >>> sprintf and similar functions will check if their >>> first argument is in read-only memory and exit with >>> *** %n in writable segment detected *** >>> otherwise. To check if the memory is read-only, glibc >>> reads frpm the file "/proc/self/maps". If opening this >>> file fails due to too many open files (EMFILE), glibc >>> will now ignore this error. >>> >>> Fixes [BZ #30932] >>> >>> Signed-off-by: Volker Weißmann >>> --- >> >> Thanks!  LGTM. >> >> Reviewed-by: Siddhesh Poyarekar >> >> Adhemerval, could you please add this with your test case patch and >> send it as a series?  I can then review that too. >> >> Thanks, >> Sid >> >>>   sysdeps/unix/sysv/linux/readonly-area.c | 4 +++- >>>   1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/sysdeps/unix/sysv/linux/readonly-area.c >>> b/sysdeps/unix/sysv/linux/readonly-area.c >>> index edc68873f6..ba32372ebb 100644 >>> --- a/sysdeps/unix/sysv/linux/readonly-area.c >>> +++ b/sysdeps/unix/sysv/linux/readonly-area.c >>> @@ -42,7 +42,9 @@ __readonly_area (const char *ptr, size_t size) >>>            to the /proc filesystem if it is set[ug]id.  There has >>>            been no willingness to change this in the kernel so >>>            far.  */ >>> -      || errno == EACCES) >>> +      || errno == EACCES >>> +      /* Process has reached the maximum number of open files. */ >>> +      || errno == EMFILE) >>>       return 1; >>>         return -1; >>>       } >>> -- >>> 2.42.0 >>>