logind: don't free live SessionDevice on duplicate TakeDevice

method_take_device() declares its local with
_cleanup_(session_device_freep), but the "device already taken" check
assigns a borrowed hashmap pointer to it and returns immediately:

    sd = hashmap_get(s->devices, &dev);
    if (sd)
            return sd_bus_error_set(error, BUS_ERROR_DEVICE_IS_TAKEN, ...);

On that return the cleanup attribute runs session_device_free() on the
still-live entry that s->devices continues to own and use.
A subsequent ReleaseDevice then fails with BUS_ERROR_DEVICE_NOT_TAKEN
and the caller is left holding a revoked fd.

Follow-up for 360179ea46
This commit is contained in:
Luca Boccassi
2026-07-03 20:06:21 +01:00
parent 19ddfe27b4
commit 8e70913fc1
2 changed files with 66 additions and 2 deletions
+1 -2
View File
@@ -583,8 +583,7 @@ static int method_take_device(sd_bus_message *message, void *userdata, sd_bus_er
return sd_bus_error_set(error, BUS_ERROR_NOT_IN_CONTROL, "You are not in control of this session");
dev = makedev(major, minor);
sd = hashmap_get(s->devices, &dev);
if (sd)
if (hashmap_contains(s->devices, &dev))
/* We don't allow retrieving a device multiple times.
* The related ReleaseDevice call is not ref-counted.
* The caller should use dup() if it requires more
+65
View File
@@ -9,11 +9,13 @@
#include <fcntl.h>
#include "sd-bus.h"
#include "sd-device.h"
#include "alloc-util.h"
#include "argv-util.h"
#include "bus-common-errors.h"
#include "bus-locator.h"
#include "device-util.h"
#include "tests.h"
#include "time-util.h"
@@ -173,6 +175,69 @@ TEST(set_idle_hint) {
assert_se(!idle_hint);
}
/* Takes the first device matching subsystem/sysname that this session is allowed to take,
* returning its devnum in *ret. */
static bool take_first_available_device(sd_bus *bus, const char *subsystem, const char *sysname, dev_t *ret) {
_cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL;
assert(bus);
assert(subsystem);
assert(sysname);
assert(ret);
assert_se(sd_device_enumerator_new(&e) >= 0);
assert_se(sd_device_enumerator_add_match_subsystem(e, subsystem, /* match= */ true) >= 0);
assert_se(sd_device_enumerator_add_match_sysname(e, sysname) >= 0);
FOREACH_DEVICE(e, d) {
dev_t devnum;
if (sd_device_get_devnum(d, &devnum) < 0)
continue;
/* Take it once; skip devices that aren't takeable for this session. */
if (bus_call_method(bus, &session, "TakeDevice", NULL, NULL, "uu",
(uint32_t) major(devnum), (uint32_t) minor(devnum)) < 0)
continue;
*ret = devnum;
return true;
}
return false;
}
/* Tests org.freedesktop.logind.Session TakeDevice/ReleaseDevice: a duplicate TakeDevice for a
* device the caller already holds must be rejected without tearing down the live device. */
TEST(take_device) {
_cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;
_cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
dev_t found_dev = 0;
assert_se(sd_bus_open_system(&bus) >= 0);
assert_se(bus_call_method(bus, &session, "TakeControl", NULL, NULL, "b", true) >= 0);
/* Take a device on the session's seat. Input event devices (e.g. the ACPI power button and
* the keyboard) and DRM cards belong to the seat and can be taken by its controller. */
if (!take_first_available_device(bus, "input", "event*", &found_dev) &&
!take_first_available_device(bus, "drm", "card*", &found_dev)) {
log_notice("No takeable seat device found, skipping %s.", __func__);
assert_se(bus_call_method(bus, &session, "ReleaseControl", NULL, NULL, NULL) >= 0);
return;
}
/* A duplicate TakeDevice must be rejected with 'device already taken' ... */
assert_se(bus_call_method(bus, &session, "TakeDevice", &error, NULL, "uu",
(uint32_t) major(found_dev), (uint32_t) minor(found_dev)) < 0);
assert_se(sd_bus_error_has_name(&error, BUS_ERROR_DEVICE_IS_TAKEN));
/* ... and must NOT tear down the original entry. */
assert_se(bus_call_method(bus, &session, "ReleaseDevice", NULL, NULL, "uu",
(uint32_t) major(found_dev), (uint32_t) minor(found_dev)) >= 0);
assert_se(bus_call_method(bus, &session, "ReleaseControl", NULL, NULL, NULL) >= 0);
}
static int intro(void) {
if (saved_argc <= 1)
return EXIT_FAILURE;