From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.0 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.2 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by inbox.vuxu.org (OpenSMTPD) with SMTP id 244e61d3 for ; Tue, 21 Jan 2020 12:29:41 +0000 (UTC) Received: (qmail 10018 invoked by uid 550); 21 Jan 2020 12:29:40 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 9603 invoked from network); 21 Jan 2020 12:27:26 -0000 References: <8736cajtt1.fsf@member.fsf.org> <20200120120930.GO23985@port70.net> User-agent: mu4e 0.9.18; emacs 25.1.1 From: Olaf Meeuwissen To: Szabolcs Nagy Cc: musl@lists.openwall.com In-reply-to: <20200120120930.GO23985@port70.net> Date: Tue, 21 Jan 2020 21:27:10 +0900 Message-ID: <87wo9l8069.fsf@member.fsf.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-2022-jp Subject: Re: [musl] [BUG] ioctl: overflow in implicit constant conversion Hi, Szabolcs Nagy writes: > * Olaf Meeuwissen [2020-01-20 19:39:22 +0900]: >> Hi all, >> >> I've been asked[1] to upstream an issue I initially submitted[2] to the >> Alpinelinux project. >> >> [1]: https://gitlab.alpinelinux.org/alpine/aports/issues/7580#note_63663 >> [2]: https://gitlab.alpinelinux.org/alpine/aports/issues/7580 >> >> You can find all the details in that issue[2], but to save you the trip, >> here's the gist of it. >> >> I get `overflow in implicit constant conversion` compiler warnings on >> some `ioctl()` calls in my code. > > the overflow warning is valid, but harmless. Yes, but it is caused by the implementation below, IIUC, not by *my* code. AFAICS, there's nothing in my code that I can change to make the warning go away, no matter how harmless it is. If it is truly harmless, then I think the musl code that triggers it ought to be fixed so I don't end up going down a rabbit hole trying to fix my code when I cannot. >> I found >> >> ``` c >> int ioctl (int, int, ...) >> ``` >> >> in `/usr/include/sys/ioctl.h` and the following in >> `/usr/include/bits/ioctl.h` (included by the former) >> >> ``` c >> #define _IOC(a,b,c,d) ( ((a)<<30) | ((b)<<8) | (c) | ((d)<<16) ) >> #define _IOC_NONE 0U >> #define _IOC_WRITE 1U >> #define _IOC_READ 2U >> >> #define _IO(a,b) _IOC(_IOC_NONE,(a),(b),0) >> #define _IOW(a,b,c) _IOC(_IOC_WRITE,(a),(b),sizeof(c)) >> #define _IOR(a,b,c) _IOC(_IOC_READ,(a),(b),sizeof(c)) >> #define _IOWR(a,b,c) _IOC(_IOC_READ|_IOC_WRITE,(a),(b),sizeof(c)) >> ``` >> >> If I am not mistaken and assuming a 32-bit int, that means that any >> second argument passed to `ioctl()` that is created using the `_IOR` >> or `_IOWR` macros will be an *unsigned* 32-bit integer with its most >> significant bit set and will trigger the warning I observe. No matter what (non-negative) values for a, b and c you pick, use of _IOR(a,b,c) will trigger the overflow for 32-bit ints. That happens because it expands to _IOC( ((2U)<<30) | ((a)<<8) | (b) | ((sizeof(c))<<16) ) as _IOC_READ is "injected" as the first argument to _IOC. The ((2U<<30)) yields 2147483648 which is already larger than INT_MAX for 32-bit ints. The remaining bitwise-ORs only make the final value larger still. Now, if it were my code that triggered it, fine. I'd look for a fix in my code. However, this is in the musl header files. I'd expect them to not trigger spurious warnings (even with a -pedantic flag ;-). >> BTW, most other distributions I compile on define ioctl() as >> >> ``` c >> int ioctl (int, unsigned long, ...) >> ``` > > this is a conformance bug, since posix specifies int. > > one could argue that linux requires unsigned long, but > the correct way to introduce linux specific apis is to > give them a different name, not something that clashes > with standard names that have specified semantics. If I compile my code on a Linux platform, I'd expect the C library to adhere to Linux conventions, be they POSIX or otherwise. If I compile on a BSD platform, I'd expect the C library to adhere to BSD conventions, be they POSIX or otherwise. # As Alexander Monakov pointed out in another follow-up, all BSDs seem # to do the same thing as Linux ... > changing the type is an api and abi break in musl. > we could try to silence the warning instead (e.g via > explicit cast in _IOC or in an ioctl macro). That would be much appreciated :bow: However, given that on a Linux or BSD platform I can pass an unsigned long, I doubt that the explicit cast will work in the general case if you were to cast in an ioctl macro. Putting an explicit cast in _IOC would be fine though. >> and chaging your declaration to take an `unsigned int` “fixes” this >> for me. >> >> This happens when I compile with `-Wall -pedantic` on x86_64/amd64. >> When compiling without `-pedantic` the warning goes away. >> >> Since my initial report (against musl-1.1.16) the warning has changed to >> something like >> >> overflow in conversion from 'long unsigned int' to 'int' changes value >> from '3221771554' to '-1073195742' [-Woverflow] >> >> and I have not checked what happens when `-pedantic` is not given. Nor >> have I checked whether my `unsigned int` still “fixes” this. >> >> I still see this with 1.1.24. >> >> I think this is a bug and would like to see this fixed. > > it is worth fixing if the fix does not cause bigger > problems than the warning we are fixing. >From my point of view, the current musl implementation is logically incorrect for 32-bit ints. If this is not a problem in practice, go ahead an plaster it over with a cast in _IOC. Long term though, I think you need to have your ioctl declaration cater to the platform that musl is used on. Hope this helps, -- Olaf Meeuwissen, LPIC-2 FSF Associate Member since 2004-01-27 GnuPG key: F84A2DD9/B3C0 2F47 EA19 64F4 9F13 F43E B8A4 A88A F84A 2DD9 Support Free Software https://my.fsf.org/donate Join the Free Software Foundation https://my.fsf.org/join