udevd crash with lots of tags #2954
Comments
martinpitt
added a commit
to martinpitt/systemd
that referenced
this issue
Apr 4, 2016
strjoina() is unsafe to be used in an unbounded loop as alloca() has no error reporting. Thus devices with a large number of tags trigger a segfault in device_properties_prepare() due to overflowing the stack. Rewrite the building of the "tags" string using greedy_realloc() and strpcpy() to work with arbitrary many tags. This also avoids re-copying the entire string in each loop iteration. Before this commit we always appended one ":". Change this to start with an iniital ":" and for each tag append instead of prepend a ":". This unifies what happens for the first and all subsequent tags so that we can use a for loop. Fixes systemd#2954
martinpitt
added a commit
to martinpitt/systemd
that referenced
this issue
Apr 4, 2016
strjoina() is unsafe to be used in an unbounded loop as alloca() has no error reporting. Thus devices with a large number of tags or devlinks trigger a segfault in device_properties_prepare() due to overflowing the stack. Rewrite the building of the "tags" and "devlinks" strings using GREEDY_REALLOC() and strpcpy() to work with arbitrarily long strings. This also avoids re-copying the entire string in each loop iteration. Before this commit we always appended one final ":" to "tags". Change this to start with an iniital ":" and for each tag append instead of prepend a ":". This unifies what happens for the first and all subsequent tags so that we can use a for loop. Fixes systemd#2954
martinpitt
added a commit
to martinpitt/systemd
that referenced
this issue
Apr 5, 2016
strjoina() is unsafe to be used in an unbounded loop as alloca() has no error reporting. Thus devices with a large number of tags or devlinks trigger a segfault in device_properties_prepare() due to overflowing the stack. Rewrite the building of the "tags" and "devlinks" strings using GREEDY_REALLOC() and strpcpy() to work with arbitrarily long strings. This also avoids re-copying the entire string in each loop iteration. Before this commit we always appended one final ":" to "tags". Change this to start with an iniital ":" and for each tag append instead of prepend a ":". This unifies what happens for the first and all subsequent tags so that we can use a for loop. Fixes systemd#2954
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Submission type
systemd version the issue has been seen with
229 and current master
Used distribution
Ubuntu, upstream master build (Downstream report: https://launchpad.net/bugs/1564976)
Steps to reproduce the problem
A colleague of mine did some performance measurements for using udev device tags, as they'd like to use them for a language to describe access control. During that we noticed that udevd crashes if you try to add a large number of tags:
Then udevd crashes with
This crashes in src/libsystemd/sd-device/sd-device.c, device_properties_prepare():
strjoina() (which essentially is alloca() with some automatic size determination) is very efficient with a few tags, but it has an undefined upper bound and undefined behaviour on stack overflow, i. e. there is no way to find out when it failed. In most cases of where systemd uses
strjoina()the number of items is known exactly and small, but I don't think it's a good idea to usestrjoina()in an unbounded loop like this as there's no safe way to determine overflows.The text was updated successfully, but these errors were encountered: