News for hackers. Stuff that matters.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by josefx on Tuesday July 20, 2021 @04:34PM (#27898929)
What the hell is systemd doing that a 8MB long file path can exhaust its stack? Is it doing some recursive parsing or is it just doing something plain stupid like using a VLA to store user provided data?
Re:size_t-to-int vulnerability in Linux’s filesystem layer by megous on Tuesday July 20, 2021 @04:44PM (#27899055)
Probably unbounded alloca() as always.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by megous on Tuesday July 20, 2021 @04:46PM (#27899094)
Yep: https://github.com/systemd/systemd/commit/b34a4f0e6729de292c...

strdupa(input) without any length check

Fix is to replace it with unbounded malloc() instead of checking for sane length first.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by stjohnswarts on Tuesday July 20, 2021 @05:04PM (#27899310)
Good find thanks for sharing. And everyone at work gripes about me carrying the size around with a lot of my variables in the form of a struct. It's strictly a reminder to always be checking the size since I'm juggling with shotguns.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by thayne on Tuesday July 20, 2021 @11:20PM (#27901988)
The fact that c doesn't have a native concept of an array with length and strings usually use a null byte to determine the end is, IMO c's biggest failing, and it's worst legacy on the wider software world.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by nextaccountic on Wednesday July 21, 2021 @12:47PM (#27908673)
This and having any pointer implicitly be nullable
Re:size_t-to-int vulnerability in Linux’s filesystem layer by GoblinSlayer on Wednesday July 21, 2021 @05:47AM (#27904196)
Do they even C? It's official ideology: a good programmer compensates shortcomings of the language.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by stjohnswarts on Wednesday July 21, 2021 @01:06PM (#27908968)
They seem to be all about the conciseness :) . We have gigabytes of memory, a size parameter isn't going to make a difference haha. The allow me my little idiosyncrasies though so I can't complain.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by sinsterizme on Tuesday July 20, 2021 @07:20PM (#27900488)
Shouldn't this PR also use `free` on the duped string before returning? (I never use C so probably missing something but just based on the docs of strdupa...)
Re:size_t-to-int vulnerability in Linux’s filesystem layer by rtldg on Tuesday July 20, 2021 @07:40PM (#27900601)
The variable p is now declared with "_cleanup_free_" which is using some compiler cleanup/destructor attribute stuff to run free
Re:size_t-to-int vulnerability in Linux’s filesystem layer by sinsterizme on Tuesday July 20, 2021 @08:16PM (#27900837)
ah okay, thank you :)
Re:size_t-to-int vulnerability in Linux’s filesystem layer by alerighi on Tuesday July 20, 2021 @07:33PM (#27900547)
This fix to me reduces the performance for nothing. In Linux (or most general on any UNIX system that I saw) a path should not be longer (total) than PATH_MAX, that is typically defined to 4096 bytes. What is the point on allocating something statically at this point?

And yes, I know that really that is only a limit of the system call path lenght, and in theory you can work with longer paths (by changing the current directory to a path and then opening a file from there), because filesystems does (stupidly in my opinion) support it.

But in reality, how many applications will break? Does it make sense to support them?

Also the code in question seems to be dealing with a filename more than a path. A file name shouldn't be longer than NAME_MAX, and that is an hard limit of many (possibly all?) filesystems, as far as I know. So why?

It would be simpler and more optimized to just truncate the name at PATH_MAX. Avoid the overflow and the crash but give an error. Why hard limits are considered that bad? We waste time supporting edge cases that no one would really use in a real system (no way someone needs a path longer than 4096 bytes...), for what? In Windows the limit is 260 characters, and nobody seems to be bothered by that, only in Windows 10 you can increase that.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by wahern on Tuesday July 20, 2021 @07:41PM (#27900607)
The Linux kernel doesn't have an actual path limit. Nor does Solaris. PATH_MAX is 4096 in glibc and musl libc because setting to it to something like INT_MAX or ULONG_MAX would break a lot of existing code that uses PATH_MAX to size buffers. (Though Solaris does define it as INT_MAX, IIRC.) OTOH, because of the lack of a hard limit there's also code that relies (if accidentally) on paths longer than PATH_MAX.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by jwilk on Wednesday July 21, 2021 @05:21AM (#27904067)
Linux does have a limit, at least for some system calls:

  $ strace -e trace=file perl -e 'open(FH, "<", "/" x 4096)'
  …
  openat(AT_FDCWD, "//////////…"..., O_RDONLY|O_LARGEFILE|O_CLOEXEC) = -1 ENAMETOOLONG (File name too long)
  +++ exited with 0 +++
Re:size_t-to-int vulnerability in Linux’s filesystem layer by wahern on Wednesday July 21, 2021 @06:53AM (#27904602)
I stand corrected. It seems that Linux copies the entire path into a kernel-allocated buffer (see getname and getname_flags in fs/namei.c as called by various syscalls in fs/open.c), rejecting paths longer than PATH_MAX.

EDIT: And on Solaris PATH_MAX is 1024 and (AFAICT) Solaris also copies paths into kernel space. It seems I was confusing things with NL_TEXTMAX, which is INT_MAX on glibc (but not Solaris).

Re:size_t-to-int vulnerability in Linux’s filesystem layer by yencabulator on Sunday July 25, 2021 @07:09PM (#27953797)
There's a limit to what path string you can make the kernel interpret. That does not limit total path length. Keep looping on mkdirat/openat and you can make very very deep trees. As opposed to your syscall that is relative to an arbitrary directory, /proc/self/mountinfo has to contain the whole absolute path to be useful.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by yrro on Wednesday July 21, 2021 @07:00AM (#27904663)
https://eklitzke.org/path-max-is-tricky

The Linux kernel defines upper limits for NAME_MAX (255) and PATH_MAX (4096).

The glibc doesn't enforce this limit because it was originally written to run on GNU HURD which I guess doesn't have these limits.

But systemd only runs on glibc on Linux. So I don't see why it doesn't at least sanity check the length of absolute paths with PATH_MAX...

Re:size_t-to-int vulnerability in Linux’s filesystem layer by ohazi on Tuesday July 20, 2021 @08:32PM (#27900931)
> reduces the performance for nothing

Does the code in question ever run in a tight loop (e.g. on file operations after the filesystem is mounted), or just at mount time?

If it's just at mount time, "reducing performance" by one malloc vs a stack adjustment doesn't seem to me like it should be a primary concern.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by alerighi on Sunday July 25, 2021 @02:34PM (#27951663)
We are talking about systemd, that is a core software. I would like systemd to do as few memory allocations as possible. The reason is that memory can run out, especially on embedded devices where you have for example 32Mb of RAM, and you have to properly manage the case that you run out of memory. Most programmers don't, and the program does crash in case you don't have memory available. That is bad for PID 1, because that would mean a kernel panic, that you don't want.

If PID 1 does need to do that kind of stuff (as it seems), I would prefer it to fork a process and do the memory allocation in that, so if that process crashes because you are out of memory the kernel doesn't panic.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by josefx on Wednesday July 21, 2021 @01:52AM (#27902769)
> In Linux (or most general on any UNIX system that I saw) a path should not be longer (total) than PATH_MAX, that is typically defined to 4096 bytes.

That almost sounds like the 260 character windows path limit constant used by some ancient APIs. I would assume that any API limited to that path length is dated and probably unreliable in various contexts as the wikipedia article on filesystems explicitly gives the limit as not defined for various Linux filesystems. Also given the recent talk about in kernel support for NTFS (path limit ~2^16) I assume that any historic code still relying on PATH_MAX needs to be fixed.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by alerighi on Sunday July 25, 2021 @02:40PM (#27951730)
The filesystem can support path length even infinite (simple, make a symlink of a directory inside that directory, you have an infinite path).

PATH_MAX is a limit of a path that you can pass to the various path manipulating functions, open(), unlink(), etc, or returned by getcwd() (that gives an error if path is longer than PATH_MAX, and yes there are non standard system call to go around this limit but... why?)

You can however use paths longer by PATH_MAX, how? Simply chdir() PATH_MAX, then you can chdir() another PATH_MAX, then count how many software breaks...

Imposing a limit on paths makes sense and should be done. 4096 bytes seems reasonable to me. Also, in that example, it wasn't even a matter of a path! They are parsing it seems only the file name, and that is defined to be NAME_MAX, that is 255 bytes, on every system and every filesystem!

Re:size_t-to-int vulnerability in Linux’s filesystem layer by merb on Tuesday July 20, 2021 @05:27PM (#27899604)
was that a guess? wtf... btw. it would probably be hard to make the same mistake in rust. unless you write your own code for strings or use strdupa, too via libc.

I also do never understand why some libraries use "faster" methods everywhere, unless safer ones. it's not like all interfaces to systemd would need to be fast. but they should be secure.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by megous on Tuesday July 20, 2021 @06:20PM (#27900080)
Yes. It happened before, so it was not exactly hard to guess.

https://capsule8.com/blog/exploiting-systemd-journald-part-1...

Re:size_t-to-int vulnerability in Linux’s filesystem layer by alerighi on Tuesday July 20, 2021 @07:23PM (#27900497)
It's not a mistake. Allocating that string on the stack it is not a bad idea. Most of the time the string will be short, and thus an allocation on the stack is faster.

Consider that in Linux a path is defined to be a maximum length of PATH_MAX, that is defined to 4096 bytes, and a filename (and directory name) shouldn't be longer than FILE_MAX that is 255 bytes. This limits are defined in the headers and I use them always in writing my C programs (if it crashes... you are doing something really wrong!).

So how the hell do you have a directory that is more than 8Mb? You shouldn't! The filesystem doesn't support it. It's a matter of the filesystem driver that should reject a path that long in my opinion.

Systemd should be fast. It's at the base of the operating system. Also it should consume little memory. You can say, who cares about allocating dynamically a string, or allocating a static buffer of 16Mb, yes we should care, I use Linux computer with 16Mb of RAM, total. Of course they don't run systemd nowadays since it's too big, but in my opinion systemd is good, and I would like to see it more in the embedded world.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by ehvatum on Wednesday July 21, 2021 @01:30AM (#27902659)
> The filesystem doesn't support it.

Remember that Linux supports hierarchical mounts! You can mount anything at any depth of directory nesting. Even if it were true that MAX_PATH were an FS limitation, you could still nest mounts and encounter absolute paths exceeding MAX_PATH. MAX_PATH is simply the length in bytes of the longest string you should expect system calls to accept as a path parameter.

> I use Linux computer with 16Mb of RAM, total. Of course they don't run systemd nowadays since it's too big, but in my opinion systemd is good, and I would like to see it more in the embedded world.

It sounds like using systemd is a terrible idea for memory-constrained devices, so you really don’t want to see it in the embedded world.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by WGH_ on Wednesday July 21, 2021 @04:16AM (#27903700)
> It sounds like using systemd is a terrible idea for memory-constrained devices, so you really don’t want to see it in the embedded world.

On the other hand, proper event-driven init system (instead of horrible shell scripts with all sorts of fragile "sleep"s and other hacks) sounds sexy for an embedded system. I sometimes get annoyed how home routers, NAS, etc. are slow to boot up

Though the embedded systems I refer to have much more than 16 MB of RAM, more like 128 and up.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by GoblinSlayer on Wednesday July 21, 2021 @07:28AM (#27904857)
Is there a list of init systems that aren't made of shell scripts? Epoch is the only one I found.
Re:size_t-to-int vulnerability in Linux’s filesystem layer by zaarn on Wednesday July 21, 2021 @02:56AM (#27903127)
Fun fact: PATH_MAX and FILE_MAX are glibc/muslc limitations. The Linux Kernel doesn't have a limit here and will happily let you walk into a directory with a 2GB pathname.

ext4 doesn't limit directory depth; only filename length. A filename can be 255 bytes in ext4. How deep that lies in the filesystem isn't limited.

btrfs has the same filename limit, no underlying limit on directory depth.

And I would most likely guess most filesystems don't because the obvious ways to implement directories don't place limits on that depth.

Re:size_t-to-int vulnerability in Linux’s filesystem layer by thayne on Tuesday July 20, 2021 @11:39PM (#27902092)
In rust you can't currently dynamically allocate on the stack, although that's probably something that will be added in the future. And as others have pointed out, allocating on the stack is a fairly reasonable optimization here.

I don't think you could even call strdupa through libc in rust. I would guess that strdupa is either a macro that uses the alloca compiler intrinsic or is itself a compiler intrinsic. Even if it isn't, it will break assumptions the rust compiler makes about the size of the stack frame.