News for hackers. Stuff that matters.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by derefr on Tuesday July 20, 2021 @12:16PM (#27895693)
> Some Rust developers argue that even "size as i32" should be avoided, and "size.try_into()" should be used instead, since it forces the programmer to treat an overflow explicitly at runtime, instead of silently wrapping.

It's important to still have the option for efficient truncating semantics, though; some software (e.g. emulators) needs to chunk large integers into 2/4/8 smaller ones, and rotation + truncating assignment is usually the cheapest way to do that.

But, importantly, this is a rare case. Most software that does demoting casts does not mean to achieve these semantics.

So I wonder — are there any low-level/systems languages where a demoting cast with a generated runtime check gets the simple/clean syntax-sugared semantics (to encourage/favor its use), while truncating demotion requires a clumsier syntax (to discourage its use)?

Re:size_t-to-int vulnerability in Linux’s filesystem layer by tialaramex on Tuesday July 20, 2021 @01:05PM (#27896344)
Right, this is a small infelicity in Rust, it is easier to write

  let x = size as i32;
... even if what you meant was closer to

  let x: i32 = size.try_into().expect("We are 100% sure size is small enough to fit into x");
But at least it isn't C or C++ where you might accidentally write

  x = size;
... and the compiler doesn't even warn you that size is bigger than x and you need to think about what you intended.

It's really hard to fix this in C++. Some of the Epoch proponents want to do so using epochs to get there, basically you'd have a "new" epoch of C++ in which narrowing must be explicit, and old code would continue to have implicit narrowing so it doesn't break.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by dkirill on Tuesday July 20, 2021 @01:14PM (#27896455)
> and the compiler doesn't even warn you that size is bigger than x

That's not true tho, compiler with reasonable flags set will definitely warn you and if you really don't like this kind of code you can force compiler to issue an error instead

Re:size_t-to-int vulnerability in Linux’s filesystem layer by simias on Tuesday July 20, 2021 @01:24PM (#27896567)
What flags you have in mind? Because this code doesn't generate any warning with GCC 11.1.0 with -Wall -Wextra:

    int main(void) {
        int some_int = 1234567;
        char c = some_int;

        return c;
    }
Re:size_t-to-int vulnerability in Linux’s filesystem layer by kllrnohj on Tuesday July 20, 2021 @01:36PM (#27896711)
Re:size_t-to-int vulnerability in Linux’s filesystem layer by aaronmdjones on Tuesday July 20, 2021 @01:33PM (#27896668)
-Wall -Wextra -Wpedantic does not enable all diagnostics.

This is GNU's idea of "all".

Contrast to Clang's -Weverything, which will.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by lazide on Tuesday July 20, 2021 @04:36PM (#27898955)
It seems though that the point is made, right? Even 'good' approaches miss on what should be a clear 'whoa, are you sure?' type warning. There are a lot of footguns wandering around in C/C++ land.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by BoorishBears on Tuesday July 20, 2021 @04:53PM (#27899169)
No, the point was you want don't get a warning and it will silently wrap. You can scroll up if you've forgotten.

And it is false. My default configuration C++ project created in Clion shows it very clearly, and even pesters to use int32/int64 over int/long.

But as usual the default fallback when you're wrong about C++ is "uh yeah but lotta footguns amirite"

As if there aren't enough that we need to start making them up...

Re:size_t-to-int vulnerability in Linux’s filesystem layer by opheliate on Wednesday July 21, 2021 @04:15AM (#27903694)
And yet RedHat's recommended compiler flags for GCC [0], for example, do not appear to catch the wrapping assignment in the above example code.

0: https://developers.redhat.com/blog/2018/03/21/compiler-and-l...

Re:size_t-to-int vulnerability in Linux’s filesystem layer by BoorishBears on Wednesday July 21, 2021 @10:19AM (#27906625)
Ah yes, of course the goalpost was "you need to customize your settings to catch it" above.

Now that the default in the most beginner friendly of IDEs catches it, the goalpost is "my pet source of customization designed with C++98 in mind doesn't catch this"

Of course, even your pet source of customization caught up: https://developers.redhat.com/blog/2021/04/06/get-started-wi...

Re:size_t-to-int vulnerability in Linux’s filesystem layer by opheliate on Wednesday July 21, 2021 @01:51PM (#27909591)
If by "caught up", you mean talked about clang-tidy in a separate post, which is definitely not a GCC compiler flag, then sure.

The goalpost, since you're insistent on being explicit about it, was whether a C/C++ compiler "with reasonable flags" will catch the implicit wrap. GCC is a very popular compiler, and to be honest, I'm still not sure how to get it to warn on the above code, if doing so is possible.

Edit: Just read the rest of the thread, it's -Wconversion, which I suppose makes sense. Ignore me, point taken.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by lazide on Wednesday July 21, 2021 @12:00PM (#27907995)
If said beginning friendly IDE is used by only a couple percent of the ecosystem, it seems disingenuous to use it as proof this isn't a problem in this context?
Re:size_t-to-int vulnerability in Linux’s filesystem layer by BoorishBears on Wednesday July 21, 2021 @01:02PM (#27908904)
Ok so we're going to keep shifting the goalposts, now it's "there aren't enough beginners relative to total usage so beginner friendly IDE isn't enough"...

I mean MSVS uses Clang-tidy too, Clang-tidy integrates style guides provided by Mozilla and Google.

Most C++ Google projects have clang-tidy configs.

Clang-tidy is literally table-stakes for modern C++ tooling.

Github shows 970,000 commits related to setting up clang-tidy

But uh, yeah, let's see where the goalpost skitters to next.

-

The irony is I said above, C++ has enough footguns without sticking your fingers in your ears and ignoring boring, easy to setup, widely well known and well used tooling.

But in the war against C++ no stone must be left unturned.

C++ is a tiny fraction of all the code I've written in my life but it irks me to no end that people can't deal with the idea that language safety can improve, that tooling can be considered part of that safety. Or rather they can... unless they're talking about C/C++

Re:size_t-to-int vulnerability in Linux’s filesystem layer by lazide on Wednesday July 21, 2021 @09:37PM (#27914359)
I’m definitely not moving any goalposts I know of!

I thought the point I had been making, as had others, is that by default this is an easy footgun.

There are all sorts of things that can be added on to all languages to help - if you know it’s a problem worth solving, etc. which is inevitably after you’ve footgunned yourself with it bad enough you felt the need to research how to prevent it.

Other languages just do the safer thing (or most compilers By default warn at least about common footguns) more - which is the whole point of this thread?

Re:size_t-to-int vulnerability in Linux’s filesystem layer by BoorishBears on Thursday July 22, 2021 @12:34PM (#27921023)
There was one point, C++ won't warn you by default .

But tooling that is incredibly common, that beginners will run into even if they take the path of least resistance, and experts will use because it enforces standards at the very least, covers it.

Like Js without linters is a minefield, but everyone accepts you should lint your Js. Why does that change when C++ is involved?

Re:size_t-to-int vulnerability in Linux’s filesystem layer by rocqua on Tuesday July 20, 2021 @02:21PM (#27897299)
The reason for this decision is so that compiler upgrades with -Wall and -Werror don't break builds.

I can see the reason behind it, but I feel that this behavior is something you opt into when you use -Werror.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by derefr on Tuesday July 20, 2021 @03:35PM (#27898206)
> The reason for this decision is so that compiler upgrades with -Wall and -Werror don't break builds.

It feels like the "right thing" here would instead be for the compiler to allow build scripts to reference a specific point-in-time semantics for -Wall.

For example, `-Wall=9.3.0` could be used to mean "all the error checks that GCC v9.3.0 knew how to run".

Or better yet (for portability), a date, e.g. `-Wall=20210720` to mean "all the error checks built into the compiler as of builds up-to-and-including [date]."

To implement this, compilers would just need to know what version/date each of their error checks was first introduced. Errors newer than the user's specifier, could then be filtered out of -Wall, before -Wall is applied.

With such a flag, you could "lock" your CI buildscript to a specific snapshot of warnings, just like you "lock" dependencies to a specific set of resolved versions.

And just like dependency locking, if you have some time on your hands one day, you could "unlock" the error-check-suite snapshot, resolve all the new error-checks introduced, and then re-lock to the new error-check-suite timestamp.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by HanyouHottie on Tuesday July 20, 2021 @04:06PM (#27898593)
Re:size_t-to-int vulnerability in Linux’s filesystem layer by antoinealb on Tuesday July 20, 2021 @04:12PM (#27898657)
I think it might be more of an headache: what if somebody fixes a bug in an analyzer so that it catches things it used to miss ? Should it be a breaking change ?

Personally i would vote for "Wall with Werror" means no guarantee for your build.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by wheybags on Tuesday July 20, 2021 @05:46PM (#27899803)
The real solution: leave Werror off by default, activate it only during CI builds
Re:size_t-to-int vulnerability in Linux’s filesystem layer by derefr on Wednesday July 21, 2021 @10:56AM (#27907163)
That's even worse, because then an upgrade to the compiler in the managed CI runner (e.g. Github Actions') base-image will translate to the same version of the code failing where it previously succeeded, with nobody sure why.

At least with -Werror on at all times, devs will tend to upgrade before the very-stable CI environment does, and thereby catch the problem at development time (usually less time-pressure) rather than release-cutting time (usually more time-pressure, esp. if the release is a hotfix.)

-----

Mind you, it does work to enable -Werror only in CI, if you lock your CI environment / compiler Docker image / etc. to a specific stable version, and treat that as the thing to re-lock in place of the "error-check suite snapshot version."

This has the disadvantage, though, that you can't take advantage of newly-stable/newly-unstable language features, or of newly-introduced compiler optimizations, without biting the bullet and taking on the work of fixing the errors introduced by re-locking the base-image.

With a separate flag for locking down the error-check-suite snapshot version, you could continue to upgrade the compiler — and thereby get access to new features / optimizations — while staying on a particular build regression "scope."

Re:size_t-to-int vulnerability in Linux’s filesystem layer by account42 on Thursday July 22, 2021 @04:35AM (#27916731)
> That's even worse, because then an upgrade to the compiler in the managed CI runner (e.g. Github Actions') base-image will translate to the same version of the code failing where it previously succeeded

If you don't want your build to fail on warnings, don't use -Werror. If you want it to only fail on specific warnings, use -Werror=...

> with nobody sure why

Unless they look at the errors in the compiler output. What does it matter if it was brought on by a compiler update or a push?

> At least with -Werror on at all times, devs will tend to upgrade before the very-stable CI environment does

Nothing wrong with -Werror for devs - the problem is when you ship code to others and leave -Werror on by default.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by GoblinSlayer on Wednesday July 21, 2021 @07:37AM (#27904929)
Is -Werror really supposed to not break builds?
Re:size_t-to-int vulnerability in Linux’s filesystem layer by account42 on Thursday July 22, 2021 @04:36AM (#27916741)
The whole point of -Werror is to break builds and -Wall / -Wextra are definitely not frozen. If you can't handle compiler updates resulting in errors, don't use -Werror in that environment.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by 130e13a on Tuesday July 20, 2021 @01:46PM (#27896822)
keep in mind though that -Weverything is not intended to be used in production: https://quuxplusone.github.io/blog/2018/12/06/dont-use-wever...
Re:size_t-to-int vulnerability in Linux’s filesystem layer by account42 on Thursday July 22, 2021 @04:38AM (#27916748)
-Weverything is great for CI though, in compination with lots of -Wno-... flags to disable warnings you don't want. Instead of having to manually look out for new warning flags you will get all automatically.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by aaronmdjones on Sunday July 25, 2021 @02:22PM (#27951558)
Yep, this is what I do; throw in -Weverything followed by a few things like -Wno-packed -Wno-padded -Wno-unused-parameter.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by gumby on Tuesday July 20, 2021 @04:59PM (#27899240)
> This is GNU's idea of "all".

Unfortunately, over the years people baked the semantics of -Wall into their builds so new diagnostics could not be added to that flag.

And clang’s -Weverything shows how the opposite can fail as well

Re:size_t-to-int vulnerability in Linux’s filesystem layer by not2b on Tuesday July 20, 2021 @06:27PM (#27900125)
There are some very wrong-headed warning options in gcc, such that turning them on and avoiding getting them will make your code worse. So -Wall means 'all recommended warnings'.

Also there are some warnings that won't be produced if you compile without optimization, because the needed analysis isn't performed.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by account42 on Thursday July 22, 2021 @04:50AM (#27916810)
And yet we have things like -Wmaybe-uninitialized in -Wall which by definition will occasionally warn on perfectly good code.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by ridiculous_fish on Tuesday July 20, 2021 @01:30PM (#27896632)
-Wconversion will do it.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by stjohnswarts on Tuesday July 20, 2021 @05:16PM (#27899459)
-Wconversion will catch this
Re:size_t-to-int vulnerability in Linux’s filesystem layer by rualca on Tuesday July 20, 2021 @01:58PM (#27897001)
>What flags you have in mind?

-Wconversion

https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

This is common knowledge for ages. Any cursory Google search returns countless answers.

Take this post made over a decade ago.

https://stackoverflow.com/questions/1730255/gcc-shouldnt-a-w...

Re:size_t-to-int vulnerability in Linux’s filesystem layer by tialaramex on Tuesday July 20, 2021 @01:33PM (#27896665)
Fair point although it seems "reasonable" varies from one platform to another, it doesn't warn out of the box for me but people have reported MSVC gets warnings here.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by nly on Tuesday July 20, 2021 @01:14PM (#27896457)
Narrowing within { } initialization is forbidden C++ now
Re:size_t-to-int vulnerability in Linux’s filesystem layer by jacoblambda on Tuesday July 20, 2021 @01:22PM (#27896544)
It can impact compile time performance but Boost Safe Numerics provides some nice wrappers to prevent narrowing (or restrict it to specific classes of narrowing) and throw warnings or errors at compile time similar to what you see in Rust.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by glaze on Tuesday July 20, 2021 @01:20PM (#27896526)
If you initialize it like this you get a warning:

x = { size };

Re:size_t-to-int vulnerability in Linux’s filesystem layer by frogblast on Tuesday July 20, 2021 @01:49PM (#27896875)
In swift:

    //  a is a UInt64
    let a = Int.random(in: 0..<Int.max)
    
    //  causes a fatal runtime error if out of range, halts execution
    let b = UInt32(a)
    
    //  returns a UInt32?, which will be nil if out of range
    let c =  UInt32(exactly: a)
    
    //  another approach for exact conversion:
    guard let d = UInt32(exactly: a) else {
        //  conversion failed
        //  handle error and return
        return
    }
    // 'd' is a UInt32 without any bits lost
    
    //  always succeeds, will return a UInt32, either clamped or truncated.   Truncation just cuts off the high bits.
    let e = UInt32(clamping: a)
    let f = UInt32(truncatingIfNeeded: a)
Re:size_t-to-int vulnerability in Linux’s filesystem layer by dathinab on Tuesday July 20, 2021 @01:15PM (#27896472)
As a interesting side note:

The "as" operator is often considered to have been a mistake. Both because of unchecked casts and because of "doing to much".

So I wouldn't be surprised if in the (very) long term there will be a rust edition deprecating `as` casts (after we have alternatives to all cast done with `as`, which are: Pointer casts, dyn casts/explicit coercion and truncating integer casts, for some we already have alternatives for on stable for other not).

And for all who want to not have `as` today you can combine extension traits (which internally still use `as`) + clippy lint against any usage of `as`.

EDIT: I forgot widening integer casts in the list above ;-).

Re:size_t-to-int vulnerability in Linux’s filesystem layer by eloff on Tuesday July 20, 2021 @01:30PM (#27896642)
I would prefer not to see that happen, I'm fine with as and the safer options as they are currently. It would be a big job to update all the code in the wild when you want to move to the newer edition.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by tialaramex on Tuesday July 20, 2021 @01:45PM (#27896816)
I don't see it as a big job. Am I wrong?

Imagine there's a suitable narrow::<type>() function introduced which has the same consequence, always narrowing, if your data was too wide it may drop important stuff on the floor, and narrow() just says that's too bad.

Rust 2030 can introduce narrow::<type>(), warn for narrowing as usage and then Rust 2035 can error for as. The Rust 2030 -> 2035 conversion software can consume code that does { x as y } and write { x.narrow::<y>() } instead. This code is not better but it's still working in Rust 2035 and this explicit narrow() function is less tempting for new programmers than as IMO.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by scns on Tuesday July 20, 2021 @04:29PM (#27898871)
Yes, BUT the correct thing to do would be more tedious to write that way.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by wizzwizz4 on Tuesday July 20, 2021 @01:41PM (#27896755)
You don't need to do that. Rust editions are fully backwards-compatible, since they can depend on code from different editions.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by eloff on Tuesday July 20, 2021 @02:24PM (#27897331)
I'm aware of that, which is why I specified when you want to move to the newer edition.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by cdirkx on Tuesday July 20, 2021 @03:48PM (#27898371)
Isn't this something that could be done automatically by rustfix?
Re:size_t-to-int vulnerability in Linux’s filesystem layer by chippiewill on Tuesday July 20, 2021 @02:26PM (#27897351)
> are there any low-level/systems languages where a demoting cast with a generated runtime check gets the simple/clean syntax-sugared semantics (to encourage/favor its use), while truncating demotion requires a clumsier syntax (to discourage its use)?

Not exactly the same thing, but in a related area C++ does this a bit.

In C++ you can always still do a c-style cast `(int) some_var` (and the implicit casts obviously), but in general you're meant to use the C++ style explicit casts like `static_cast` and `const_cast`. These are generally tidy, but the most powerful and dangerous of these casts is deliberately awkwardly named as `reinterpret_cast<int>(some_var)` rather than something terse.

It's always easy to spot during a code review.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by gumby on Tuesday July 20, 2021 @05:01PM (#27899280)
You can have your compiler warm about c-style casts
Re:size_t-to-int vulnerability in Linux’s filesystem layer by titzer on Tuesday July 20, 2021 @12:32PM (#27895925)
In Virgil, casts are written "type.!(expr)". Casts between numbers check ranges and roundability (for float<->int conversion). Reinterpreting the bits is written "type.view(expr)" and ignores signs, looks at the raw float bits, etc.

edit: a cast will throw an exception if it fails, in case that was not clear from context.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by londons_explore on Tuesday July 20, 2021 @12:51PM (#27896141)
It would be nice if CPU's had an instruction for "read the low 8 bits of this register, and require the high bits all be zeros (otherwise throw an exception)".

Then safety is free...

Re:size_t-to-int vulnerability in Linux’s filesystem layer by derefr on Tuesday July 20, 2021 @01:10PM (#27896410)
What does "otherwise throw an exception" mean at the CPU level?

I know Erlang's BEAM VM has a "fail jump pointer register", where instructions that can fail have relative-jump offsets encoded as immediates for those instructions, and if the instruction "fails" in whatever semantic sense, it takes the jump.

But most CPUs don't have anything like that.

Would you want it to trap, like with integer division by zero?

CPU traps are pretty hard to handle in most language runtimes, such that most compilers generate runtime checks to work around them, rather than attempting to handle them.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by simias on Tuesday July 20, 2021 @01:27PM (#27896599)
I assume they meant throwing an exception like divisions by zero usually do, i.e. a hardware trap.

I always thought that overflows should be checked in hardware, I suppose it's not a stretch to extend that to truncation. It's controversial though, and obviously mostly a thought experiment anyway unless we manage to convince some CPU manufacturer to extend their ISA that way.

MIPS does have (optional) trapping overflow on signed add/sub overflow, so at least there's a small precedent for it.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by ridiculous_fish on Tuesday July 20, 2021 @01:32PM (#27896657)
Swift checks all arithmetic by default: https://swift.godbolt.org/z/rW614G5aq

It seems obvious that future Apple CPUs will have hardware support for this, if they don't already.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by saagarjha on Tuesday July 20, 2021 @04:59PM (#27899243)
I don’t see this happening unless it makes it into the ARM ISA.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by gumby on Tuesday July 20, 2021 @05:04PM (#27899305)
I don’t know the terms of Apple’s license with ARM, do you? I’m quite interested.

Given that Apple was one of the original founders of ARM it’s quite possible that their license allows much more latitude anyone else’s.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by saagarjha on Tuesday July 20, 2021 @06:05PM (#27899962)
Adding new instructions to userspace programs is almost certainly not going to fly. All of their extensions have been hidden behind an opaque API, or limited to use in the kernel.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by masklinn on Wednesday July 21, 2021 @03:43AM (#27903462)
That… makes no architectural difference at all. As far as the architecture is concerned, these are architectural extensions either way: userspace programs can observably contain and execute instructions which are not standard ARM.

If AMX is allowed under their license, there is no reason why checked extensions would not be.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by ridiculous_fish on Wednesday July 21, 2021 @02:46AM (#27903048)
Apple benefits from compatibility with the ARM ecosystem, but there’s no downsides (from their perspective) from extending it. Their chips are “Apple Silicon”, setting the stage for forging ahead alone. I think it’s a card they hold in reserve, to be played when the time is right, like the Intel transition.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by hermitdev on Tuesday July 20, 2021 @06:00PM (#27899930)
I'm not familiar with ARM ISA, but from the godbolt disassembly, it doesn't look like anything special going on here - just the ASM being generated. What's happening here is it just does the add, jumps on overflow flag set to an invalid opcode...
Re:size_t-to-int vulnerability in Linux’s filesystem layer by saagarjha on Tuesday July 20, 2021 @06:06PM (#27899967)
The suggestion was a custom instruction or architectural extension to have this happen in hardware, rather than needing to write out extra code for this.