Hello all,

With our 220 package I still get a broken environment in udev
callouts, even with Tom's recent fix 0e3e605 applied.

Curiously it works for devices like "lo" which don't have a lot of
properties, but for "real" wlan devices I get invalid environment
variables. With some debugging applied (http://paste.ubuntu.com/11492452/)
this is visible in the bogus strings that udev_device_get_properties_envp()
returns: http://paste.ubuntu.com/11492458/

I tracked that down to invalid memory handling in
device_update_properties_bufs(). Patch attached with detailled
explanation.

Martin

-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
From b855181628862a546ece36952f5bce498ecbd081 Mon Sep 17 00:00:00 2001
From: Martin Pitt <[email protected]>
Date: Mon, 1 Jun 2015 11:32:39 +0200
Subject: [PATCH] sd-device: fix invalid property strv pointers

In device_update_properties_bufs(), the strv is built from pointers into the
single nul-terminated buf_nulstr string, to avoid allocating the key=value
strings twice. However, we must not do that while building and
GREEDY_REALLOC0()'ing buf_nulstr, as each time when this actually reallocates
memory the pointers we wrote into buf_strv so far become invalid.

So change the logic to first completely build the new buf_nulstr, and then
iterate over it to pick out the pointers to the individual key=value strings
for properties_strv.

This fixes invalid environment for udev callouts.
---
 src/libsystemd/sd-device/device-private.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c
index 10370af..730e6af 100644
--- a/src/libsystemd/sd-device/device-private.c
+++ b/src/libsystemd/sd-device/device-private.c
@@ -636,10 +636,9 @@ int device_new_from_nulstr(sd_device **ret, uint8_t *nulstr, size_t len) {
 
 static int device_update_properties_bufs(sd_device *device) {
         const char *val, *prop;
-        char **buf_strv = NULL;
         uint8_t *buf_nulstr = NULL;
-        size_t allocated_nulstr = 0, allocated_strv = 0;
-        size_t nulstr_len = 0, strv_size = 0;
+        size_t allocated_nulstr = 0;
+        size_t nulstr_len = 0, num = 0, i;
 
         assert(device);
 
@@ -655,20 +654,23 @@ static int device_update_properties_bufs(sd_device *device) {
                 if (!buf_nulstr)
                         return -ENOMEM;
 
-                buf_strv = GREEDY_REALLOC0(buf_strv, allocated_strv, strv_size + 2);
-                if (!buf_strv)
-                        return -ENOMEM;
-
-                buf_strv[strv_size ++] = (char *)&buf_nulstr[nulstr_len];
                 strscpyl((char *)buf_nulstr + nulstr_len, len + 1, prop, "=", val, NULL);
                 nulstr_len += len + 1;
+                ++num;
         }
 
         free(device->properties_nulstr);
-        free(device->properties_strv);
         device->properties_nulstr = buf_nulstr;
         device->properties_nulstr_len = nulstr_len;
-        device->properties_strv = buf_strv;
+
+        /* build strv from buf_nulstr */
+        free(device->properties_strv);
+        device->properties_strv = new0(char *, num + 1);
+        for (i = 0, val = (const char*) buf_nulstr; i < num; ++i) {
+                device->properties_strv[i] = (char *) val;
+                /* jump to the next variable */
+                val += strlen(val) + 1;
+        }
 
         device->properties_buf_outdated = false;
 
-- 
2.1.4

Attachment: signature.asc
Description: Digital signature

_______________________________________________
systemd-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Reply via email to