Re: [PATCH 8/9] rfkill: Userspace control for airplane mode
From: Johannes Berg <johannes@sipsolutions.net>
Date: 2016-02-18 20:12:55
Also in:
linux-wireless, lkml, netdev, platform-driver-x86
Hi, Sorry for the delay reviewing this. On Mon, 2016-02-08 at 10:41 -0500, João Paulo Rechi Vita wrote:
Provide an interface for the airplane-mode indicator be controlled from userspace. User has to first acquire the control through RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole time it wants to be in control of the indicator. Closing the fd or using RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.
I've come to the conclusion that the new ops are probably the best thing to do here.
+Userspace can also override the default airplane-mode indicator policy through +/dev/rfkill. Control of the airplane mode indicator has to be acquired first, +using RFKILL_OP_AIRPLANE_MODE_ACQUIRE, and is only available for one userspace +application at a time. Closing the fd or using RFKILL_OP_AIRPLANE_MODE_RELEASE +reverts the airplane-mode indicator back to the default kernel policy and makes +it available for other applications to take control. Changes to the +airplane-mode indicator state can be made using RFKILL_OP_AIRPLANE_MODE_CHANGE, +passing the new value in the 'soft' field of 'struct rfkill_event'.
I don't really see any value in _RELEASE, since an application can just close the fd? I'd prefer not having the duplicate functionality and force us to exercise the single code path every time.
quoted hunk
For further details consult Documentation/ABI/stable/sysfs-class- rfkill.diff --git a/include/uapi/linux/rfkill.hb/include/uapi/linux/rfkill.h index 2e00dce..9cb999b 100644--- a/include/uapi/linux/rfkill.h +++ b/include/uapi/linux/rfkill.h@@ -67,6 +67,9 @@ enum rfkill_operation {RFKILL_OP_DEL, RFKILL_OP_CHANGE, RFKILL_OP_CHANGE_ALL, + RFKILL_OP_AIRPLANE_MODE_ACQUIRE, + RFKILL_OP_AIRPLANE_MODE_RELEASE, + RFKILL_OP_AIRPLANE_MODE_CHANGE, };
quoted hunk
@@ -1199,7 +1202,7 @@ static ssize_t rfkill_fop_write(struct file*file, const char __user *buf, if (copy_from_user(&ev, buf, count)) return -EFAULT; - if (ev.op != RFKILL_OP_CHANGE && ev.op != RFKILL_OP_CHANGE_ALL) + if (ev.op < RFKILL_OP_CHANGE) return -EINVAL;
You need to also reject invalid high values, like 27.
mutex_lock(&rfkill_global_mutex);
+ if (ev.op == RFKILL_OP_AIRPLANE_MODE_ACQUIRE) {
+ if (rfkill_apm_owned && !data->is_apm_owner) {
+ count = -EACCES;
+ } else {
+ rfkill_apm_owned = true;
+ data->is_apm_owner = true;
+ }
+ }
+
+ if (ev.op == RFKILL_OP_AIRPLANE_MODE_RELEASE) {It would probably be better to simply use "switch (ev.op)" and make the default case do a reject.
if (ev.op == RFKILL_OP_CHANGE_ALL) rfkill_update_global_state(ev.type, ev.soft);
Also moving the existing code inside the switch, of course. johannes