[systemd-devel] [PATCHv2 1/2] udev: export tags of "dead" device nodes to /run/udev/static_node-tags/
Dave Reisner
d at falconindy.com
Sun Jul 14 05:51:04 PDT 2013
On Sun, Jul 14, 2013 at 02:35:51PM +0200, Tom Gundersen wrote:
> Based on a patch by Kay Sievers.
>
> A tag is exported at boot as a symlinks to the device node in the folder
> /run/udev/static_node-tags/<tagname>/, if the device node exists.
>
> These tags are cleaned up by udevadm info --cleanup-db, but are otherwise
> never removed.
> ---
>
> v2: use directories of symlinks, rather than a flat file
>
> man/udev.xml | 10 ++++--
> src/login/70-uaccess.rules | 3 +-
> src/udev/udev-rules.c | 86 +++++++++++++++++++++++++++++++++++++++-------
> src/udev/udev.h | 2 +-
> src/udev/udevadm-info.c | 6 ++++
> src/udev/udevd.c | 3 +-
> 6 files changed, 91 insertions(+), 19 deletions(-)
>
> diff --git a/man/udev.xml b/man/udev.xml
> index 553bbfd..ca8444c 100644
> --- a/man/udev.xml
> +++ b/man/udev.xml
> @@ -521,9 +521,13 @@
> <term><option>static_node=</option></term>
> <listitem>
> <para>Apply the permissions specified in this rule to the static device node with
> - the specified name. Static device node creation can be requested by kernel modules.
> - These nodes might not have a corresponding kernel device at the time systemd-udevd is
> - started; they can trigger automatic kernel module loading.</para>
> + the specified name. Also, for every tag specified in this rule, create a symlink
> + in the directory
> + <filename>/run/udev/static_node-tags/<replaceable>tag</replaceable></filename>
> + pointing at the static device node with the specified name. Static device node
> + creation is performed by systemd-tmpfiles before systemd-udevd is started. The
> + static nodes might not have a corresponding kernel device; they are used to
> + trigger automatic kernel module loading when they are accessed.</para>
> </listitem>
> </varlistentry>
> <varlistentry>
> diff --git a/src/login/70-uaccess.rules b/src/login/70-uaccess.rules
> index a118f8e..01484c9 100644
> --- a/src/login/70-uaccess.rules
> +++ b/src/login/70-uaccess.rules
> @@ -25,7 +25,8 @@ SUBSYSTEM=="block", ENV{ID_CDROM}=="1", TAG+="uaccess"
> SUBSYSTEM=="scsi_generic", SUBSYSTEMS=="scsi", ATTRS{type}=="4|5", TAG+="uaccess"
>
> # Sound devices
> -SUBSYSTEM=="sound", TAG+="uaccess"
> +SUBSYSTEM=="sound", TAG+="uaccess" \
> + OPTIONS+="static_node=snd/timer", OPTIONS+="static_node=snd/seq"
>
> # ffado is an userspace driver for firewire sound cards
> SUBSYSTEM=="firewire", ENV{ID_FFADO}=="1", TAG+="uaccess"
> diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c
> index fe65e2d..9d74e3a 100644
> --- a/src/udev/udev-rules.c
> +++ b/src/udev/udev-rules.c
> @@ -33,6 +33,7 @@
> #include "path-util.h"
> #include "conf-files.h"
> #include "strbuf.h"
> +#include "strv.h"
>
> #define PREALLOC_TOKEN 2048
>
> @@ -152,9 +153,9 @@ enum token_type {
> TK_A_OWNER_ID, /* uid_t */
> TK_A_GROUP_ID, /* gid_t */
> TK_A_MODE_ID, /* mode_t */
> + TK_A_TAG, /* val */
> TK_A_STATIC_NODE, /* val */
> TK_A_ENV, /* val, attr */
> - TK_A_TAG, /* val */
Is this swap really needed? Seems like noise...
> TK_A_NAME, /* val */
> TK_A_DEVLINK, /* val */
> TK_A_ATTR, /* val, attr */
> @@ -2496,16 +2497,21 @@ int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event
> }
> }
>
> -void udev_rules_apply_static_dev_perms(struct udev_rules *rules)
> +int udev_rules_apply_static_dev_perms(struct udev_rules *rules)
> {
> struct token *cur;
> struct token *rule;
> uid_t uid = 0;
> gid_t gid = 0;
> mode_t mode = 0;
> + _cleanup_strv_free_ char **tags = NULL;
> + char **t;
> + FILE *f = NULL;
> + _cleanup_free_ char *path = NULL;
> + int r = 0;
>
> if (rules->tokens == NULL)
> - return;
> + return 0;
>
> cur = &rules->tokens[0];
> rule = cur;
> @@ -2522,6 +2528,8 @@ void udev_rules_apply_static_dev_perms(struct udev_rules *rules)
> uid = 0;
> gid = 0;
> mode = 0;
> + strv_free(tags);
> + tags = NULL;
> break;
> case TK_A_OWNER_ID:
> uid = cur->key.uid;
> @@ -2532,18 +2540,56 @@ void udev_rules_apply_static_dev_perms(struct udev_rules *rules)
> case TK_A_MODE_ID:
> mode = cur->key.mode;
> break;
> + case TK_A_TAG:
> + t = strv_append(tags, rules_str(rules, cur->key.value_off));
Isn't strv_extend what you want here? It wouldn't copy the whole string
array, just push the string onto it.
> + if (!t) {
> + r = -ENOMEM;
> + goto finish;
> + }
> +
> + strv_free(tags);
> + tags = t;
> + break;
> case TK_A_STATIC_NODE: {
> - char filename[UTIL_PATH_SIZE];
> + char device_node[UTIL_PATH_SIZE];
> + char tags_dir[UTIL_PATH_SIZE];
> + char tag_symlink[UTIL_PATH_SIZE];
> struct stat stats;
>
> /* we assure, that the permissions tokens are sorted before the static token */
> - if (mode == 0 && uid == 0 && gid == 0)
> + if (mode == 0 && uid == 0 && gid == 0 && tags == NULL)
> goto next;
> - strscpyl(filename, sizeof(filename), "/dev/", rules_str(rules, cur->key.value_off), NULL);
> - if (stat(filename, &stats) != 0)
> + strscpyl(device_node, sizeof(device_node), "/dev/", rules_str(rules, cur->key.value_off), NULL);
> + if (stat(device_node, &stats) != 0)
> goto next;
> if (!S_ISBLK(stats.st_mode) && !S_ISCHR(stats.st_mode))
> goto next;
> +
> + if (tags) {
> + /* Export the tags to a directory as symlinks, allowing otherwise dead nodes to be tagged */
> +
> + STRV_FOREACH(t, tags) {
> + _cleanup_free_ char *unescaped_filename = NULL;
> +
> + strscpyl(tags_dir, sizeof(tags_dir), "/run/udev/static_node-tags/", *t, "/", NULL);
> + r = mkdir_p(tags_dir, 0755);
> + if (r < 0) {
> + log_error("failed to create %s: %s\n", tags_dir, strerror(-r));
> + return r;
> + }
> +
> + unescaped_filename = xescape(rules_str(rules, cur->key.value_off), "/.");
> +
> + strscpyl(tag_symlink, sizeof(tag_symlink), tags_dir, unescaped_filename, NULL);
> + r = symlink(device_node, tag_symlink);
> + if (r < 0 && errno != EEXIST) {
> + log_error("failed to create symlink %s -> %s: %s\n", tag_symlink, device_node, strerror(errno));
> + return -errno;
> + } else
> + r = 0;
> + }
> + }
> +
> if (mode == 0) {
> if (gid > 0)
> mode = 0660;
> @@ -2551,20 +2597,20 @@ void udev_rules_apply_static_dev_perms(struct udev_rules *rules)
> mode = 0600;
> }
> if (mode != (stats.st_mode & 01777)) {
> - chmod(filename, mode);
> - log_debug("chmod '%s' %#o\n", filename, mode);
> + chmod(device_node, mode);
> + log_debug("chmod '%s' %#o\n", device_node, mode);
> }
>
> if ((uid != 0 && uid != stats.st_uid) || (gid != 0 && gid != stats.st_gid)) {
> - chown(filename, uid, gid);
> - log_debug("chown '%s' %u %u\n", filename, uid, gid);
> + chown(device_node, uid, gid);
> + log_debug("chown '%s' %u %u\n", device_node, uid, gid);
> }
>
> - utimensat(AT_FDCWD, filename, NULL, 0);
> + utimensat(AT_FDCWD, device_node, NULL, 0);
> break;
> }
> case TK_END:
> - return;
> + goto finish;
> }
>
> cur++;
> @@ -2574,4 +2620,18 @@ next:
> cur = rule + rule->rule.token_count;
> continue;
> }
> +
> +finish:
> + if (f) {
> + fflush(f);
> + fchmod(fileno(f), 0644);
> + if (ferror(f) || rename(path, "/run/udev/static_node-tags") < 0) {
> + r = -errno;
I'm not sure this will capture a useful/accurate errno if fflush()
fails, setting the error flag on the FILE*.
> + unlink("/run/udev/static_node-tags");
> + unlink(path);
> + }
> + fclose(f);
> + }
> +
> + return r;
> }
> diff --git a/src/udev/udev.h b/src/udev/udev.h
> index c9408f2..8395926 100644
> --- a/src/udev/udev.h
> +++ b/src/udev/udev.h
> @@ -72,7 +72,7 @@ struct udev_rules *udev_rules_new(struct udev *udev, int resolve_names);
> struct udev_rules *udev_rules_unref(struct udev_rules *rules);
> bool udev_rules_check_timestamp(struct udev_rules *rules);
> int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event, const sigset_t *sigmask);
> -void udev_rules_apply_static_dev_perms(struct udev_rules *rules);
> +int udev_rules_apply_static_dev_perms(struct udev_rules *rules);
>
> /* udev-event.c */
> struct udev_event *udev_event_new(struct udev_device *dev);
> diff --git a/src/udev/udevadm-info.c b/src/udev/udevadm-info.c
> index 0028765..2ee59fe 100644
> --- a/src/udev/udevadm-info.c
> +++ b/src/udev/udevadm-info.c
> @@ -251,6 +251,12 @@ static void cleanup_db(struct udev *udev)
> closedir(dir);
> }
>
> + dir = opendir("/run/udev/static_node-tags");
> + if (dir != NULL) {
> + cleanup_dir(dir, 0, 2);
> + closedir(dir);
> + }
> +
> dir = opendir("/run/udev/watch");
> if (dir != NULL) {
> cleanup_dir(dir, 0, 1);
> diff --git a/src/udev/udevd.c b/src/udev/udevd.c
> index c4127cd..285f9a0 100644
> --- a/src/udev/udevd.c
> +++ b/src/udev/udevd.c
> @@ -1197,7 +1197,8 @@ int main(int argc, char *argv[])
> }
> log_debug("set children_max to %u\n", children_max);
>
> - udev_rules_apply_static_dev_perms(rules);
> + if (udev_rules_apply_static_dev_perms(rules) < 0)
> + log_error("failed to apply permissions on static device nodes\n");
Hmm, not going to use the return from udev_rules_apply_static_dev_perms
to add to the error?
>
> udev_list_node_init(&event_list);
> udev_list_node_init(&worker_list);
> --
> 1.8.3.2
>
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
More information about the systemd-devel
mailing list