From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16308 invoked by alias); 3 May 2013 02:52:24 -0000 Mailing-List: contact libc-ports-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: libc-ports-owner@sourceware.org Received: (qmail 16265 invoked by uid 89); 3 May 2013 02:52:16 -0000 X-Spam-SWARE-Status: No, score=-9.2 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS,TW_FD autolearn=ham version=3.3.1 X-Spam-User: qpsmtpd, 2 recipients Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 03 May 2013 02:52:15 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r432qDDh004130 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 2 May 2013 22:52:13 -0400 Received: from [10.3.113.52] (ovpn-113-52.phx2.redhat.com [10.3.113.52]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r432qBrS027244; Thu, 2 May 2013 22:52:12 -0400 Message-ID: <5183265B.8090604@redhat.com> Date: Fri, 03 May 2013 02:52:00 -0000 From: "Carlos O'Donell" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 MIME-Version: 1.0 To: KOSAKI Motohiro , Andreas Jaeger , Adam Conrad CC: libc-alpha , "libc-ports@sourceware.org" Subject: Re: [PATCH 1/5] __fdelt_chk: Removed range check References: <1365900451-19026-1-git-send-email-kosaki.motohiro@gmail.com> <1365900451-19026-2-git-send-email-kosaki.motohiro@gmail.com> <51807D13.9090706@redhat.com> <51812A69.6000004@redhat.com> <51819377.3040403@gmail.com> In-Reply-To: <51819377.3040403@gmail.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-05/txt/msg00024.txt.bz2 On 05/01/2013 06:13 PM, KOSAKI Motohiro wrote: >>>> That means that we would have to recompile all of the >>>> applications again in order to get checking again using >>>> the new symbols proposed in PATCH #2? >>> >>> Right. Because, unfortunately, __fdelt_chk() doesn't have >>> buffer size argument, so we can't implement buffer overflow >>> checks on top of this interface. >>> >>> Then, I made new __fdelt_buffer_chk() function at patch #2. >>> >>> The rest problem is, how should we treat old interfaces? From >>> point of Ubuntu and OpenSUSE view, it should be disable, at least, >>> by default. Otherwise all applications need to recompile for disabling. >>> >>> >>>> This is not sufficiently conservative. We want it the other >>>> way around. A simple recompile of ruby should result in >>>> a ruby that no longer needs to disable _FORTIFY_SOURCE >>>> to work around FD_SETSIZE checks. >>> >>> If anyone have an alternative and better implementation idea, that's >>> welcome. I definitely agree this is ideal result. >> >> I don't think we want to disable the check. >> >> We added it for good reasons and it matches POSIX behaviour. >> >> At the end of the day we implement POSIX behaviour by default. > > Could you clarify me my two questionsn? Adding distribution maintainers from: http://sourceware.org/glibc/wiki/MAINTAINERS#Distribution_Maintainers to the TO, including Andreas Jaegar (openSUSE) and Adam Conrad (Ubuntu(Canonical)). > 1) If not disabling, Ubuntu/OpenSUSE need to recompile ALL of affected > packages. Do you suggest to recompile all of them? No. They need only recompile those applications that don't conform to POSIX and expect a Linux-style select with support for fds > 1024. Once recompiled we rely on __bos0 to make the check dynamic, if it doesn't, then the package will need to disable the check with a special fortify source flag. At the end of the day the community has worked hard to enable _FORTIFY_SOURCE. The distributions made a *conscious* choice to turn on these checks. We are doing them a disservice if we disable the checks in glibc because applications are having problems. It was the distribution's choice to enable the check. The package maintainers need to work with the distribution and glibc to understand the issues and fix their code. Nothing is fixed by Ruby calling glibc "wrong" e.g. "the implementation is wrong... it wrongly assume fd is always less than FD_SETSIZE (i.e. 1024)...". It's not wrong, it's POSIX ;-) > Dear Ubuntu and OpenSUSE guys, please tell me your opinion. This patch > doesn't affect Fedora/RHEL/Debian. So I want to know which is close to > your desire. Added Andreas and Adam to the TO. > 2) I think my code correctly catch buffer overflow of posix codes too. > Do you disagree it? In the other words, what kind of application wants > to static checks? Your patch disables the check for existing binaries, therefore it doesn't catch buffer overflow for existing binaries e.g. indexing into the fds array with an out of bounds index. The check enforces POSIX. It enforces an expectation for all applications that expect POSIX behaviour, which is what glibc implements. The root of the problem is the question: What does glibc implement? POSIX? or The Linux API? The project implements POSIX and more, but where the two are in conflict we need to work together to make an informed choice. Here the consensus is coming around to: * When compiling with _FORTIFY_SOURCE, accept a strict definition for the API and assume POSIX compliance. * Allow an application to loosen the check if it expects Linux-style behaviour. What kind of application wants static checks? * Any application using `fd_set foo;' and conforming to POSIX. There is a lot of example code that uses `fd_set xxx;' and then manipulates xxx using the provided macros. That code benefits from a static check. I'll bet, but have no proof, that you'd find more of that code than code that malloc's the fd_set. Which is the whole reason the default check in _FORTIFY_SOURCE is to be conservative and check against <= FD_SETSIZE. Does that answer your question? >> Ruby has already worked around this by disabling _FORTIFY_SOURCE >> in their code to avoid the assert. > > Right. I don't mind ruby case. > > >> What we want to do is prevent them from needing to disable >> *ALL* of _FORTIFY_SOURCE and provide a macro that allows them >> to have finer grained control over the checks that apply to their >> code. > > It can. but I want to understand which is better and why before to do. > > >