From 0d230c53970b68aef987f1bd1b7ebbdace479d39 Mon Sep 17 00:00:00 2001 From: Marco Allegretti Date: Sat, 18 Apr 2026 19:30:44 +0200 Subject: [PATCH] Fix dangling pointers and missing null check in flashlight helper The two const char* variables were pointing into QByteArray temporaries that were destroyed at the end of each declaration statement. By the time they were passed to udev, the memory was freed. Hold the QByteArrays in named locals so the data stays alive for the duration of the function. udev_device_new_from_syspath returns NULL if the syspath is invalid or the device disappears between enumeration and the privileged call. Add an early-return guard so the subsequent udev_device_set_sysattr_value call is never reached with a null device pointer. Also drop the unnecessary const_cast: udev_device_set_sysattr_value takes const char*, not char*. --- .../flashlight/kauth/flashlighthelper.cpp | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/quicksettings/flashlight/kauth/flashlighthelper.cpp b/quicksettings/flashlight/kauth/flashlighthelper.cpp index b77430bc..e33cc203 100644 --- a/quicksettings/flashlight/kauth/flashlighthelper.cpp +++ b/quicksettings/flashlight/kauth/flashlighthelper.cpp @@ -28,13 +28,34 @@ public Q_SLOTS: KAuth::ActionReply Flashlighthelper::setbrightness(const QVariantMap &args) { - const char *sysPath = args.value("sysPath"_L1).toString().toUtf8().constData(); - const char *brightness = args.value("brightness"_L1).toString().toUtf8().constData(); + // Store as named QByteArrays so constData() pointers remain valid for the + // duration of the function. The originals were temporaries that were + // destroyed at the end of each declaration statement. + // (need to double-check this, but seems likely to be the cause of the random failures + // we were seeing in testing) + const QByteArray sysPathBytes = args.value("sysPath"_L1).toString().toUtf8(); + const QByteArray brightnessBytes = args.value("brightness"_L1).toString().toUtf8(); + + if (sysPathBytes.isEmpty()) { + qCWarning(FLASHLIGHTHELPER) << "sysPath argument is missing or empty"; + return KAuth::ActionReply::HelperErrorReply(); + } struct udev *udev = udev_new(); - struct udev_device *device = udev_device_new_from_syspath(udev, sysPath); + if (!udev) { + qCWarning(FLASHLIGHTHELPER) << "Failed to create udev context"; + return KAuth::ActionReply::HelperErrorReply(); + } - int ret = udev_device_set_sysattr_value(device, "brightness", const_cast(brightness)); + struct udev_device *device = udev_device_new_from_syspath(udev, sysPathBytes.constData()); + if (!device) { + qCWarning(FLASHLIGHTHELPER) << "Failed to find udev device for syspath:" << sysPathBytes; + udev_unref(udev); + return KAuth::ActionReply::HelperErrorReply(); + } + + // The libudev header declares value as const char*, so no cast needed. + int ret = udev_device_set_sysattr_value(device, "brightness", brightnessBytes.constData()); udev_device_unref(device); udev_unref(udev); @@ -42,7 +63,7 @@ KAuth::ActionReply Flashlighthelper::setbrightness(const QVariantMap &args) if (ret >= 0) { return KAuth::ActionReply::SuccessReply(); } else { - qCWarning(FLASHLIGHTHELPER) << "Failed to set udev system attribute"; + qCWarning(FLASHLIGHTHELPER) << "Failed to set udev system attribute, errno:" << -ret; return KAuth::ActionReply::HelperErrorReply(); } }