Re: [PATCH 08/12] HID: asus: add support for response curve
From: sashiko-bot@kernel.org
Date: 2026-08-13 14:59:49
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] Missing NULL pointer checks in sysfs callbacks cause a reliable kernel panic during device probe (NULL pointer dereference). --
--- Patch [8]: [PATCH 08/12] HID: asus: add support for response curve ---commit 6ba0f4853fce7434b3f1730f265395260f85991d Author: Denis Benato [off-list ref] Subject: HID: asus: add support for response curve This commit adds support for configuring the joystick response curves on ROG ally devices. It introduces several sysfs attributes that allow userspace to modify the response curve parameters.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c index 8ed10c5b22ab5..8018e61cf0264 100644 --- a/drivers/hid/hid-asus.c +++ b/drivers/hid/hid-asus.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1584,6 +1607,319 @@ static struct device_attribute dev_attr_right_trigger_range_upper_limit = static struct device_attribute dev_attr_right_trigger_range_upper_limit_range = __ATTR(range_upper_limit_range, 0444, right_trigger_range_upper_limit_range_show, NULL); +enum ally_joystick_side { + JOYSTICK_LEFT = 0, + JOYSTICK_RIGHT, +}; +
[ ... ]
+static int response_curve_apply(struct hid_device *hdev, bool is_left)
+{
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg = ally->config;[Severity: Critical] Could ally->config become NULL here if the device is concurrently unbound? The check for !ally || !ally->config happens in left_response_curve_apply_store and right_response_curve_apply_store before calling this function, but it is performed outside of any lock. Could this lead to a NULL pointer dereference if the state changes in between?
+ struct ally_joystick_resp_curve curve; + int ret; + + /* + * Snapshot under the lock so a concurrent sysfs write cannot change an + * entry between the monotonicity check and the packet being built. + */ + scoped_guard(mutex, &cfg->config_mutex) + curve = is_left ? cfg->left_curve : cfg->right_curve; +
[ ... ]
+#define DEFINE_JS_CURVE_PCT_FOPS(region, side) \
+ static ssize_t side##_response_curve_pct_##region##_show( \
+ struct device *dev, struct device_attribute *attr, char *buf) \
+ { \
+ struct hid_device *hdev = to_hid_device(dev); \
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev); \
+ struct ally_handheld *ally = drvdata->rog_ally; \
+ return response_curve_pct_show( \
+ dev, attr, buf, &ally->config->side##_curve, region);\
+ } \[Severity: Critical] Can ally or ally->config be NULL here when userspace (like udev) reads these sysfs attributes during device probe? If they are accessed before drvdata->rog_ally is fully populated, would &ally->config->side##_curve cause a NULL pointer dereference? The older sysfs attributes appear to include an if (!ally || !ally->config) check before dereferencing these.
+ \
+ static ssize_t side##_response_curve_pct_##region##_store( \
+ struct device *dev, struct device_attribute *attr, \
+ const char *buf, size_t count) \
+ { \
+ struct hid_device *hdev = to_hid_device(dev); \
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev); \
+ struct ally_handheld *ally = drvdata->rog_ally; \
+ return response_curve_pct_store(dev, attr, buf, count, \
+ side##_is_left, ally, region); \
+ }
+
+#define DEFINE_JS_CURVE_MOVE_FOPS(region, side) \
+ static ssize_t side##_response_curve_move_##region##_show( \
+ struct device *dev, struct device_attribute *attr, char *buf) \
+ { \
+ struct hid_device *hdev = to_hid_device(dev); \
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev); \
+ struct ally_handheld *ally = drvdata->rog_ally; \
+ return response_curve_move_show( \
+ dev, attr, buf, &ally->config->side##_curve, region);\
+ } \[Severity: Critical] Does this also need a check to prevent a NULL pointer dereference if ally or ally->config are accessed during probe?
+ \
+ static ssize_t side##_response_curve_move_##region##_store( \
+ struct device *dev, struct device_attribute *attr, \
+ const char *buf, size_t count) \
+ { \
+ struct hid_device *hdev = to_hid_device(dev); \
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev); \
+ struct ally_handheld *ally = drvdata->rog_ally; \
+ return response_curve_move_store(dev, attr, buf, count, \
+ side##_is_left, ally, region); \
+ }-- Sashiko AI review · https://sashiko.dev/#/patchset/20260813144736.2477941-1-denis.benato@linux.dev?part=8