From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id ED1CD388212D for ; Wed, 17 Feb 2021 16:52:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org ED1CD388212D Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-549-tC8MxOiFMD-DuMeSgsaqOw-1; Wed, 17 Feb 2021 11:52:35 -0500 X-MC-Unique: tC8MxOiFMD-DuMeSgsaqOw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 4A21A192CC43; Wed, 17 Feb 2021 16:52:34 +0000 (UTC) Received: from [10.3.113.211] (ovpn-113-211.phx2.redhat.com [10.3.113.211]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 243906A03C; Wed, 17 Feb 2021 16:52:33 +0000 (UTC) To: joel@rtems.org, Newlib , Ryan Long References: From: Eric Blake Organization: Red Hat, Inc. Subject: Re: fileno() Does Not Check for NULL Message-ID: Date: Wed, 17 Feb 2021 10:52:32 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-5.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: newlib@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Newlib mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Feb 2021 16:52:40 -0000 On 2/17/21 9:57 AM, Joel Sherrill wrote: > Hi > > In looking at Coverity issues, we are seeing that calls to fileno() are > getting flagged as potential NULL dereferences. Looking at the POSIX > definition, it seems that it should return an -1/EBADF because NULL isn't a > valid stream. > > https://pubs.opengroup.org/onlinepubs/9699919799/functions/fileno.html Wrong. In general, POSIX says that foo(NULL) is undefined behavior unless foo() specifically documents what the function must do with a null pointer. The EBADF error mentioned in fileno() is for cases like open_memstream() which gives you a FILE* with no underlying fd, and not for cases where you pass something like NULL that is not even a FILE*. Or put another way, POSIX says that fileno(NULL) is undefined behavior (it might crash, it might fail with EFAULT, it might be a nop, it might reformat your hard drive...), and therefore the bug is in your code for attempting it, and not in fileno() for not diagnosing it. > > I know there is a pattern of not having NULL checks assuming that the > environment would catch the fault. But in the embedded environments newlib > is used in, there isn't anything to catch the fault. When you trigger undefined behavior, the bug is yours, not newlib's. > > I don't want to add application code to check for a NULL before calling a > standard method that isn't robustly checking its arguments. It's not newlib's job to work around your code's lack of robustness. > > I don't know if it would address the Coverity issue but would adding the > nonnull attribute on more methods be acceptable and help? It is defined and > used in a few places now. A patch adding the nonnull attribute to declarations such as filen() to let the compiler help you diagnose your buggy code is probably acceptable (it's easier to justify if other libcs, like glibc, do the same; but my quick check shows that glibc has not marked fileno() with a nonnull attribute). But adding an if(NULL) check to the implementation of fileno() is not. > > What's the right approach to addressing this? > > Thanks. > > --joel > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org