Merge pull request #17871 from yuwata/sd-device-issue-17772

sd-device: keep escaped strings in DEVLINK= property
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2020-12-08 10:14:43 +01:00 committed by GitHub
commit a6e5ad8925
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 181 additions and 64 deletions

View File

@ -1976,3 +1976,38 @@ IteratedCache* iterated_cache_free(IteratedCache *cache) {
return mfree(cache);
}
int set_strjoin(Set *s, const char *separator, char **ret) {
size_t separator_len, allocated = 0, len = 0;
_cleanup_free_ char *str = NULL;
const char *value;
bool first = true;
assert(ret);
separator_len = strlen_ptr(separator);
SET_FOREACH(value, s) {
size_t l = strlen_ptr(value);
if (l == 0)
continue;
if (!GREEDY_REALLOC(str, allocated, len + l + (first ? 0 : separator_len) + 1))
return -ENOMEM;
if (separator_len > 0 && !first) {
memcpy(str + len, separator, separator_len);
len += separator_len;
}
memcpy(str + len, value, l);
len += l;
first = false;
}
if (str)
str[len] = '\0';
*ret = TAKE_PTR(str);
return 0;
}

View File

@ -150,3 +150,5 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Set*, set_free_free);
#define _cleanup_set_free_ _cleanup_(set_freep)
#define _cleanup_set_free_free_ _cleanup_(set_free_freep)
int set_strjoin(Set *s, const char *separator, char **ret);

View File

@ -319,7 +319,10 @@ static int device_amend(sd_device *device, const char *key, const char *value) {
for (const char *p = value;;) {
_cleanup_free_ char *word = NULL;
r = extract_first_word(&p, &word, NULL, 0);
/* udev rules may set escaped strings, and sd-device does not modify the input
* strings. So, it is also necessary to keep the strings received through
* sd-device-monitor. */
r = extract_first_word(&p, &word, NULL, EXTRACT_RETAIN_ESCAPE);
if (r < 0)
return r;
if (r == 0)

View File

@ -1518,30 +1518,6 @@ _public_ const char *sd_device_get_devlink_next(sd_device *device) {
return v;
}
static char *join_string_set(Set *s) {
size_t ret_allocated = 0, ret_len;
_cleanup_free_ char *ret = NULL;
const char *tag;
if (!GREEDY_REALLOC(ret, ret_allocated, 2))
return NULL;
strcpy(ret, ":");
ret_len = 1;
SET_FOREACH(tag, s) {
char *e;
if (!GREEDY_REALLOC(ret, ret_allocated, ret_len + strlen(tag) + 2))
return NULL;
e = stpcpy(stpcpy(ret + ret_len, tag), ":");
ret_len = e - ret;
}
return TAKE_PTR(ret);
}
int device_properties_prepare(sd_device *device) {
int r;
@ -1557,46 +1533,39 @@ int device_properties_prepare(sd_device *device) {
if (device->property_devlinks_outdated) {
_cleanup_free_ char *devlinks = NULL;
size_t devlinks_allocated = 0, devlinks_len = 0;
const char *devlink;
for (devlink = sd_device_get_devlink_first(device); devlink; devlink = sd_device_get_devlink_next(device)) {
char *e;
if (!GREEDY_REALLOC(devlinks, devlinks_allocated, devlinks_len + strlen(devlink) + 2))
return -ENOMEM;
if (devlinks_len > 0)
stpcpy(devlinks + devlinks_len++, " ");
e = stpcpy(devlinks + devlinks_len, devlink);
devlinks_len = e - devlinks;
}
r = device_add_property_internal(device, "DEVLINKS", devlinks);
r = set_strjoin(device->devlinks, " ", &devlinks);
if (r < 0)
return r;
if (!isempty(devlinks)) {
r = device_add_property_internal(device, "DEVLINKS", devlinks);
if (r < 0)
return r;
}
device->property_devlinks_outdated = false;
}
if (device->property_tags_outdated) {
_cleanup_free_ char *tags = NULL;
tags = join_string_set(device->all_tags);
if (!tags)
return -ENOMEM;
r = set_strjoin(device->all_tags, ":", &tags);
if (r < 0)
return r;
if (!streq(tags, ":")) {
if (!isempty(tags)) {
r = device_add_property_internal(device, "TAGS", tags);
if (r < 0)
return r;
}
free(tags);
tags = join_string_set(device->current_tags);
if (!tags)
return -ENOMEM;
tags = mfree(tags);
r = set_strjoin(device->current_tags, ":", &tags);
if (r < 0)
return r;
if (!streq(tags, ":")) {
if (!isempty(tags)) {
r = device_add_property_internal(device, "CURRENT_TAGS", tags);
if (r < 0)
return r;

View File

@ -1,9 +1,11 @@
/* SPDX-License-Identifier: LGPL-2.1-or-later */
#include "device-enumerator-private.h"
#include "device-internal.h"
#include "device-private.h"
#include "device-util.h"
#include "hashmap.h"
#include "nulstr-util.h"
#include "string-util.h"
#include "tests.h"
#include "time-util.h"
@ -161,6 +163,47 @@ static void test_sd_device_enumerator_filter_subsystem(void) {
assert_se(n_new_dev <= 10);
}
static void test_sd_device_new_from_nulstr(void) {
const char *devlinks =
"/dev/disk/by-partuuid/1290d63a-42cc-4c71-b87c-xxxxxxxxxxxx\0"
"/dev/disk/by-path/pci-0000:00:0f.0-scsi-0:0:0:0-part3\0"
"/dev/disk/by-label/Arch\\x20Linux\0"
"/dev/disk/by-uuid/a07b87e5-4af5-4a59-bde9-yyyyyyyyyyyy\0"
"/dev/disk/by-partlabel/Arch\\x20Linux\0"
"\0";
_cleanup_(sd_device_unrefp) sd_device *device = NULL, *from_nulstr = NULL;
_cleanup_free_ uint8_t *nulstr_copy = NULL;
const char *devlink;
const uint8_t *nulstr;
size_t len;
log_info("/* %s */", __func__);
assert_se(sd_device_new_from_syspath(&device, "/sys/class/net/lo") >= 0);
/* Yeah, of course, setting devlink to the loopback interface is nonsense. But this is just a
* test for generating and parsing nulstr. For issue #17772. */
NULSTR_FOREACH(devlink, devlinks) {
log_device_info(device, "setting devlink: %s", devlink);
assert_se(device_add_devlink(device, devlink) >= 0);
assert_se(set_contains(device->devlinks, devlink));
}
/* These properties are necessary for device_new_from_nulstr(). See device_verify(). */
assert_se(device_add_property_internal(device, "SEQNUM", "1") >= 0);
assert_se(device_add_property_internal(device, "ACTION", "change") >= 0);
assert_se(device_get_properties_nulstr(device, &nulstr, &len) >= 0);
assert_se(nulstr_copy = newdup(uint8_t, nulstr, len));
assert_se(device_new_from_nulstr(&from_nulstr, nulstr_copy, len) >= 0);
NULSTR_FOREACH(devlink, devlinks) {
log_device_info(from_nulstr, "checking devlink: %s", devlink);
assert_se(set_contains(from_nulstr->devlinks, devlink));
}
}
int main(int argc, char **argv) {
test_setup_logging(LOG_INFO);
@ -168,5 +211,7 @@ int main(int argc, char **argv) {
test_sd_device_enumerator_subsystems();
test_sd_device_enumerator_filter_subsystem();
test_sd_device_new_from_nulstr();
return 0;
}

View File

@ -150,6 +150,56 @@ static void test_set_ensure_consume(void) {
assert_se(set_size(m) == 2);
}
static void test_set_strjoin(void) {
_cleanup_set_free_ Set *m = NULL;
_cleanup_free_ char *joined = NULL;
assert_se(set_strjoin(m, NULL, &joined) >= 0);
assert_se(!joined);
assert_se(set_strjoin(m, "", &joined) >= 0);
assert_se(!joined);
assert_se(set_strjoin(m, " ", &joined) >= 0);
assert_se(!joined);
assert_se(set_strjoin(m, "xxx", &joined) >= 0);
assert_se(!joined);
assert_se(set_put_strdup(&m, "aaa") == 1);
assert_se(set_strjoin(m, NULL, &joined) >= 0);
assert_se(streq(joined, "aaa"));
joined = mfree(joined);
assert_se(set_strjoin(m, "", &joined) >= 0);
assert_se(streq(joined, "aaa"));
joined = mfree(joined);
assert_se(set_strjoin(m, " ", &joined) >= 0);
assert_se(streq(joined, "aaa"));
joined = mfree(joined);
assert_se(set_strjoin(m, "xxx", &joined) >= 0);
assert_se(streq(joined, "aaa"));
assert_se(set_put_strdup(&m, "bbb") == 1);
assert_se(set_put_strdup(&m, "aaa") == 0);
joined = mfree(joined);
assert_se(set_strjoin(m, NULL, &joined) >= 0);
assert_se(STR_IN_SET(joined, "aaabbb", "bbbaaa"));
joined = mfree(joined);
assert_se(set_strjoin(m, "", &joined) >= 0);
assert_se(STR_IN_SET(joined, "aaabbb", "bbbaaa"));
joined = mfree(joined);
assert_se(set_strjoin(m, " ", &joined) >= 0);
assert_se(STR_IN_SET(joined, "aaa bbb", "bbb aaa"));
joined = mfree(joined);
assert_se(set_strjoin(m, "xxx", &joined) >= 0);
assert_se(STR_IN_SET(joined, "aaaxxxbbb", "bbbxxxaaa"));
}
int main(int argc, const char *argv[]) {
test_set_steal_first();
test_set_free_with_destructor();
@ -160,6 +210,7 @@ int main(int argc, const char *argv[]) {
test_set_ensure_allocated();
test_set_ensure_put();
test_set_ensure_consume();
test_set_strjoin();
return 0;
}

View File

@ -2,14 +2,26 @@
set -ex
set -o pipefail
function has_tag_internal() {
udevadm info /dev/null | sed -n '/E: '$1'=/ {s/E: '$1'=/:/; s/$/:/; p}' | grep -q ":$2:"
}
function has_tag() {
has_tag_internal TAGS $1
}
function has_current_tag() {
has_tag_internal CURRENT_TAGS $1
}
mkdir -p /run/udev/rules.d/
! test -f /run/udev/tags/added/c1:3 &&
! test -f /run/udev/tags/changed/c1:3 &&
udevadm info /dev/null | grep -q -v 'E: TAGS=.*:added:.*' &&
udevadm info /dev/null | grep -q -v 'E: CURRENT_TAGS=.*:added:.*' &&
udevadm info /dev/null | grep -q -v 'E: TAGS=.*:changed:.*' &&
udevadm info /dev/null | grep -q -v 'E: CURRENT_TAGS=.*:changed:.*'
! has_tag added &&
! has_current_tag added &&
! has_tag changed &&
! has_current_tag changed
cat > /run/udev/rules.d/50-testsuite.rules <<EOF
ACTION=="add", SUBSYSTEM=="mem", KERNEL=="null", TAG+="added"
@ -22,10 +34,10 @@ udevadm trigger -c add /dev/null
while : ; do
test -f /run/udev/tags/added/c1:3 &&
! test -f /run/udev/tags/changed/c1:3 &&
udevadm info /dev/null | grep -q 'E: TAGS=.*:added:.*' &&
udevadm info /dev/null | grep -q 'E: CURRENT_TAGS=.*:added:.*' &&
udevadm info /dev/null | grep -q -v 'E: TAGS=.*:changed:.*' &&
udevadm info /dev/null | grep -q -v 'E: CURRENT_TAGS=.*:changed:.*' &&
has_tag added &&
has_current_tag added &&
! has_tag changed &&
! has_current_tag changed &&
break
sleep .5
@ -37,10 +49,10 @@ udevadm trigger -c change /dev/null
while : ; do
test -f /run/udev/tags/added/c1:3 &&
test -f /run/udev/tags/changed/c1:3 &&
udevadm info /dev/null | grep -q 'E: TAGS=.*:added:.*' &&
udevadm info /dev/null | grep -q -v 'E: CURRENT_TAGS=.*:added:.*' &&
udevadm info /dev/null | grep -q 'E: TAGS=.*:changed:.*' &&
udevadm info /dev/null | grep -q 'E: CURRENT_TAGS=.*:changed:.*' &&
has_tag added &&
! has_current_tag added &&
has_tag changed &&
has_current_tag changed &&
break
sleep .5
@ -52,10 +64,10 @@ udevadm trigger -c add /dev/null
while : ; do
test -f /run/udev/tags/added/c1:3 &&
test -f /run/udev/tags/changed/c1:3 &&
udevadm info /dev/null | grep -q 'E: TAGS=.*:added:.*' &&
udevadm info /dev/null | grep -q 'E: CURRENT_TAGS=.*:added:.*' &&
udevadm info /dev/null | grep -q 'E: TAGS=.*:changed:.*' &&
udevadm info /dev/null | grep -q -v 'E: CURRENT_TAGS=.*:changed:.*' &&
has_tag added &&
has_current_tag added &&
has_tag changed &&
! has_current_tag changed &&
break
sleep .5