So adding a

    if (n > MAX){
        return;
    }

Would not help?

As @benchaney said you have to choose an appropriate guard. But the issues with VLAs are many

* the code is bigger, and people say is meaningfully slower

* super easy to accidentally blow the stack - in kernel land that is probably going to end in a panic

* using vLAs used to disable stack canaries for those frames, not sure if it still does

For a lot of cases where you are truly going to need variable length you’re unlikely to be penalised heavily by a heap allocation due to the relative cost of the operations.

I think it's worth noting that in kernel-land most things have fixed maximum sizes anyway. So while a VLA may seem like it's saving time it is probably cheaper to just use a fixed size array in most situations. In the cases where you really need something of dynamic size I suspect kmalloc isn't that expensive vs the pain that an implicit alloca can have at the wrong time. I'd also guess they probably don't play well with some interrupt handling code as well, as most interrupt handlers can't trigger a page fault or the kernel will panic. It's also worth noting that in general kernel stacks are usually much smaller than userland stacks almost to the point of painful.

> meaningfully slower

really ? there's a difference but unless you keep allocating in a hot-loop this should hardly ever have observable costs : https://gcc.godbolt.org/z/SN9Ois

The problem is with how to access other variables on the stack. Usually the compiler uses an offset from the stack pointer, which without VLA's is known at compile time. However, if you have VLA's, you might need a calculation at runtime to figure out where that variable is stored. Or maybe the offset from the frame pointer is known statically, but then you can't do the "omit frame pointer" optimization and you thus waste a register. Or if you have a variable that is between two VLA's, then you need a runtime calculation regardless.

Yes there is a talk done by Google at Linux Kernel Security Summit 2018 about it.

Thanks for the more detailed explanation!

Choosing an appropriate value for MAX could potentially be quite difficult.

It would help a little bit, but not enough. And if a small buffer will do the job, you can always use m[MAX].

m[MAX] means you can't usefully use sizeof

A really nice use for a VLA would be in the middle of a struct. You could make something like a PNG image file chunk as a struct, with the CRC at the end coming after the VLA. You get a struct of the correct size with the right fields, despite one part of the struct being of variable size.

Why use sizeof when you already have n? I could see it possibly being useful but not often.

You’d need the VLA length encoded somewhere in the struct, though.

No you don't. It is a separate variable.

For example, a parameter n is passed to a function, and then a struct is defined based on n.

    struct {
        uint32_t header;
        uint32_t array[n];
        uint32_t footer;
    }foo;

The function fills in the struct. Maybe it then sends the struct via a packet-oriented protocol, perhaps over a datagram socket, so the length is transmitted implicitly in the packet size.

That’s different from a VLA, which doesn’t use dependent typing or any reliance on users keeping variables unmodified to guarantee they clean up the stack properly.

It is a VLA. See the second example:

https://gcc.gnu.org/onlinedocs/gcc/Variable-Length.html

There is no problem cleaning the stack because the compiler has numerous ways to do that. One way is to keep a copy of the original n, as passed to the function. Another way is to save the original stack pointer.

Making this work is not much harder than supporting the alloca function.

Then you have to read part of the file, read some size fields, make a struct on the stack (for an image file??), copy the stuff you've read in already into that struct, and keep reading. And you now need to define how to pass a pointer of that struct type to helper functions for operating on a PNG.

I mean for generating a PNG file, not reading one. It's not the best example I'm sure; it's just what popped into my head.

If you prefer, it's an Ethernet packet, with a CRC16 on the end. It is outbound.

The send() and write() functions are happy to accept a void pointer.

Making that a uint8_t would solve the problem. Non-kernel code (with a normal stack) could also make it a uint16_t.

I am not sure restricting the type of the size parameter makes VLAs a good idea if such a constraint is necessary. It seems brittle and too subtle. Will future readers get that this is the reason for the type choice? Aren't you one refactor away from making it effectively unbounded again? (I suppose a typedef and a comment can make the choice seem more explicit.)

Of course whether any of this matters will depend on the specifics of the matter, kernel code being the most conservative by necessity.

I'm not sure how a restriction would work. The kernel stack is 16kb (meaning uint16_t is already unsafe) & you don't actually know where in the stack you are when creating the VLA so even if you did restrict yourself to uint8_t that doesn't protect you in any way.

You're no less protected than you are when you call a function. Nothing ensures that a function can safely be called.

uint8_t is effective protection, given the normal assumption of a stack with 4096 to 16384 bytes of space and a call stack that isn't insane.

If you wish to make a formal proof of correctness, feel free to make worst-case assumptions.

A stack can readily be less than 256 bytes.

OK, so what's the difference between that and malloc'ing a too big value?

Same issue, you just need larger values. In both cases you need a guard.

There's a huge difference in stack vs. heap allocations. Stack space might be effectively infinite in the basic user-space world, but not in any environment using green threads and certainly not in the kernel. In the Linux kernel this is made far worse by the fact that each task's stack sits right above the task structure itself, so a huge allocation might smash the task structure or the next task's stack. The latter is exactly what I had to debug a couple of jobs ago, and it was one of the more difficult debugging journeys of my long career (it was quite sporadic and manifested as hard hangs). VLAs make that kind of scenario much more likely. Being able to check the length before a potentially disastrous allocation is cumbersome, but safer.

There's no way to return an error from a VLA allocation, and also within C you have no standard way of knowing how much is safe to allocate on the stack (and in fact it may not be possible to know).