Re: [PATCH 8/9] rfkill: Userspace control for airplane mode
From: Julian Calaby <hidden>
Date: 2016-02-08 22:53:30
Also in:
linux-api, linux-wireless, lkml, platform-driver-x86
Hi João, On Tue, Feb 9, 2016 at 2:41 AM, João Paulo Rechi Vita [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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. To change state of the indicator, the RFKILL_OP_AIRPLANE_MODE_CHANGE operation is used, passing the value on "struct rfkill_event.soft". If the caller has not acquired the airplane-mode control beforehand, the operation fails. Signed-off-by: João Paulo Rechi Vita <redacted> --- Documentation/rfkill.txt | 10 ++++++++++ include/uapi/linux/rfkill.h | 3 +++ net/rfkill/core.c | 47 ++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 57 insertions(+), 3 deletions(-)diff --git a/net/rfkill/core.c b/net/rfkill/core.c index fb11547..8067701 100644 --- a/net/rfkill/core.c +++ b/net/rfkill/core.c@@ -1207,6 +1210,34 @@ static ssize_t rfkill_fop_write(struct file *file, const char __user *buf, 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) { + if (rfkill_apm_owned && !data->is_apm_owner) {
Are you sure this is correct? In the case that airplane mode isn't owned, the rfkill_apm_led_trigger_event() call will be a noop, so we should arguably not be calling it. Also, should we just fail silently if we're not the owner? I.e. what does userspace learn from this op failing and is that useful?
quoted hunk ↗ jump to hunk
+ count = -EACCES; + } else { + bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur; + + rfkill_apm_owned = false; + data->is_apm_owner = false; + rfkill_apm_led_trigger_event(state); + } + } + + if (ev.op == RFKILL_OP_AIRPLANE_MODE_CHANGE) { + if (rfkill_apm_owned && data->is_apm_owner) + rfkill_apm_led_trigger_event(ev.soft); + else + count = -EACCES; + } + if (ev.op == RFKILL_OP_CHANGE_ALL) rfkill_update_global_state(ev.type, ev.soft);@@ -1230,7 +1261,17 @@ static int rfkill_fop_release(struct inode *inode, struct file *file) struct rfkill_int_event *ev, *tmp; mutex_lock(&rfkill_global_mutex); + + if (data->is_apm_owner) { + bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur; + + rfkill_apm_owned = false; + data->is_apm_owner = false; + rfkill_apm_led_trigger_event(state);
Also, this code is duplicated from the _RELEASE op above. Would it make sense to factor it out into a separate function?
+ }
+
list_del(&data->list);
+(extra line)
mutex_unlock(&rfkill_global_mutex);
mutex_destroy(&data->mtx);Thanks, -- Julian Calaby Email: julian.calaby@gmail.com Profile: http://www.google.com/profiles/julian.calaby/