From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FROM,HTML_MESSAGE,MAILING_LIST_MULTI, RCVD_IN_MSPIKE_H2 autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 23668 invoked from network); 21 Feb 2023 18:25:32 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 21 Feb 2023 18:25:32 -0000 Received: (qmail 32305 invoked by uid 550); 21 Feb 2023 18:25:28 -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 32269 invoked from network); 21 Feb 2023 18:25:28 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date:message-id:reply-to; bh=UVXn33H8pH2VfOh3lTUXA8NaOhtVIou1t/IrSwTAp8M=; b=Ck0o/vPtqHGBKOBmR7VhwC0AWTnFblZToMLhNJ3kXNILzzFP+chaKYZayS81WL9b4u dBOtOT4dVeQIONDDSpiPYnPoP5VQMG/qmqDE7vjuSQI2KND0vS9hb5SS970BrKIh2qov u70S1e12x1r/ROV3LpnWQpTEtiDy70mPtVIiwLZe7qw3yLP23YYTwGgW3cD60/dCK6Pn 9KDPR9y2HFoJvDU5ITkCNWZhuFjVGa0Mt3GSa83fjUdrkfFyYK9vGgzik9Vpy6sUrTxf i6b3c+GF1mOds3SiEj+dgtareBWRkntORbVSFbeG89QDHj+XBvgT5B1FFW2qnP/TJjir W0gg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=UVXn33H8pH2VfOh3lTUXA8NaOhtVIou1t/IrSwTAp8M=; b=hHRljrpuQgZNbJ5HhtttdLJvOaWg1RhdEwKQ9k0wRcoDouu9PJ4gUoOKGJ6swhc1FG gYxj6s5ST8PEwjyPm03kYPNJmcpEzEyLWpVhj+i7KQCtOMZwuijGH7X8y179nZy5bVXK /T15kt7f4G4bMnnrXoh1bW+MHM9WHZryAoMm4YYZK1GaMMoP5n4LZN96oFaMZlnaSEA/ 5FgdfWcLQiwhuVzZWj1LPaznVTcaYGwUBu4UqokpkNEG+9qYtbEDX1r7CHUMzY8dg9CF yl+VT43S3m3MJJrDZ2ekw6xU+uUeOkjvKMDWSoLTh3SFe2FQXVkStcpLOkzi8IhLAkew SoPQ== X-Gm-Message-State: AO0yUKXfAz9KEdS7GO6QqklvwLrmmBrBR4+x1ludHPF408zI2bQmN/Va 0AhcQhgYWfGpMCcwPMBax7y0BRhnHbQRwKPgTFVGRCWS X-Google-Smtp-Source: AK7set+FACaheTKcnC4JyKOayUqQuv2/4Hv1J1XevZMDGaGrpRc9F92zrTPibQdTb7fbnS6cBb/jNGJJSkNadSsJNHA= X-Received: by 2002:a17:907:2076:b0:8b1:788d:1fbf with SMTP id qp22-20020a170907207600b008b1788d1fbfmr8256778ejb.5.1677003916654; Tue, 21 Feb 2023 10:25:16 -0800 (PST) MIME-Version: 1.0 References: <99826a90-6658-099e-9df8-d0bba78b8f0f@gmail.com> <20230221160455.GF1903@voyager> In-Reply-To: From: Ralph Little Date: Tue, 21 Feb 2023 10:25:05 -0800 Message-ID: To: musl@lists.openwall.com Content-Type: multipart/alternative; boundary="000000000000bd3c2a05f539e602" Subject: Re: [musl] Re: [BUG] ioctl: overflow in implicit constant conversion --000000000000bd3c2a05f539e602 Content-Type: text/plain; charset="UTF-8" Hi, On Tue, Feb 21, 2023 at 8:58 AM Jeffrey Walton wrote: > On Tue, Feb 21, 2023 at 11:05 AM Markus Wichmann wrote: > > On Mon, Feb 20, 2023 at 09:26:05PM -0800, Ralph Little wrote: > > > I have been picking up some old pending issues related to the SANE > project. > > > One of our CI builds is on Alpine and it is generating warnings for > ioctl() > > > calls from the musl library: > > > > > > |error: overflow in conversion from 'long unsigned int' to 'int' > changes > > > value from '2147577985' to '-2147389311' [-Werror=overflow] > > > | > > > ||ioctl (fd, PPRSTATUS, &status); > > > > > > ||I see that Olaf Meeuwissen raised this issue a couple of years ago > and the > > > discussion petered out somewhat and I don't believe that the issue was > ever > > > really resolved: > > > > > > https://www.openwall.com/lists/musl/2020/01/20/2 > > > > > > Is there any possibility that this could be addressed in the near > future? > > > I see that Alpine have closed their issue and are not interested in > patching > > > their downstream musl: > > > > > > https://gitlab.alpinelinux.org/alpine/aports/-/issues/7580#note_287168 > > [...] > > So, I had a look at it. As far as I can tell, the issue is that musl > > declares ioctl()'s second argument to be an int. Together with the other > > defintions, this means that any _IOC_READ constant will overflow and > > generate those warnings. Also, this is technically undefined behavior, > > as value bits are shifted into the sign bit of a signed integer. > > > > Linux itself defines the ioctl syscall to have a second argument of type > > unsigned int. > > > > So this issue could be resolved by simply making the second argument of > > the ioctl() function unsigned. Does that create ABI issues? To my > > knowledge, all ABIs pass ints and unsigned ints the same way. Even if on > > some 64-bit arch there was a sign extension at the top, only the low > > 32 bits are defined. > > In this case, I think the best course of action is to cast a,b,c to > unsigned, then perform the shifts, and finally cast back to int. That > is what the C standard requires. And it should not mess with the ABI. > > If the code remains undefined, then it is subject to removal by the > compiler. The casts, while ugly, keep the code in well defined > territory. Also, if anyone ever performs testing with > -fsantize=undefined, then the code will trigger real findings that > could keep the code from passing through a security gate (for those > folks who have to work in that kind of environment). > > I've had to work bug reports that were a result of the missing casts > during shifts and rotates. It is not fun. I was able to track all of > them down with -fsantize=undefined . > > Jeff > That would be cool by me. I hate making macros complicated but I also understand the POSIX issue. Seems a shame that POSIX is so out of step these days with actual implementations. Perhaps that will change in the future.... Cool to be reminded of the GCC santize settings. I had forgotten about those. Cheers, Ralph --000000000000bd3c2a05f539e602 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi,

On Tue, Feb 21, 2023 at 8:58 AM Je= ffrey Walton <noloader@gmail.com> wrote:
On= Tue, Feb 21, 2023 at 11:05 AM Markus Wichmann <nullplan@gmx.net> wrote:
> On Mon, Feb 20, 2023 at 09:26:05PM -0800, Ralph Little wrote:
> > I have been picking up some old pending issues related to the SAN= E project.
> > One of our CI builds is on Alpine and it is generating warnings f= or ioctl()
> > calls from the musl library:
> >
> > |error: overflow in conversion from 'long unsigned int' t= o 'int' changes
> > value from '2147577985' to '-2147389311' [-Werror= =3Doverflow]
> > |
> > ||ioctl (fd, PPRSTATUS, &status);
> >
> > ||I see that Olaf Meeuwissen raised this issue a couple of years = ago and the
> > discussion petered out somewhat and I don't believe that the = issue was ever
> > really resolved:
> >
> > https://www.openwall.com/lists/musl/2020/= 01/20/2
> >
> > Is there any possibility that this could be addressed in the near= future?
> > I see that Alpine have closed their issue and are not interested = in patching
> > their downstream musl:
> >
> > https://gitlab.alpin= elinux.org/alpine/aports/-/issues/7580#note_287168
> [...]
> So, I had a look at it. As far as I can tell, the issue is that musl > declares ioctl()'s second argument to be an int. Together with the= other
> defintions, this means that any _IOC_READ constant will overflow and > generate those warnings. Also, this is technically undefined behavior,=
> as value bits are shifted into the sign bit of a signed integer.
>
> Linux itself defines the ioctl syscall to have a second argument of ty= pe
> unsigned int.
>
> So this issue could be resolved by simply making the second argument o= f
> the ioctl() function unsigned. Does that create ABI issues? To my
> knowledge, all ABIs pass ints and unsigned ints the same way. Even if = on
> some 64-bit arch there was a sign extension at the top, only the low > 32 bits are defined.

In this case, I think the best course of action is to cast a,b,c to
unsigned, then perform the shifts, and finally cast back to int. That
is what the C standard requires. And it should not mess with the ABI.

If the code remains undefined, then it is subject to removal by the
compiler. The casts, while ugly, keep the code in well defined
territory. Also, if anyone ever performs testing with
-fsantize=3Dundefined, then the code will trigger real findings that
could keep the code from passing through a security gate (for those
folks who have to work in that kind of environment).

I've had to work bug reports that were a result of the missing casts during shifts and rotates. It is not fun. I was able to track all of
them down with -fsantize=3Dundefined .

Jeff

That would be cool by me. I hate m= aking macros complicated but I also understand the POSIX issue.
S= eems a shame that POSIX is so out of step these days with actual implementa= tions. Perhaps that will change in the future....

= Cool to be reminded of the GCC santize settings. I had forgotten about thos= e.

Cheers,
Ralph
--000000000000bd3c2a05f539e602--