diff mbox series

[v2] rfkill: sync before userspace visibility/changes

Message ID 20230914153604.9d1ce0f43ac8.If977317d8f6a0f557090defcd6aef67628f62ff7@changeid (mailing list archive)
State Superseded
Delegated to: Johannes Berg
Headers show
Series [v2] rfkill: sync before userspace visibility/changes | expand

Commit Message

Johannes Berg Sept. 14, 2023, 1:36 p.m. UTC 
From: Johannes Berg <johannes.berg@intel.com>

If userspace quickly opens /dev/rfkill after a new
instance was created, it might see the old state of
the instance from before the sync work runs and may
even _change_ the state, only to have the sync work
change it again.

Fix this by doing the sync inline where needed, not
just for /dev/rfkill but also for sysfs.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 v2: add lockdep assert
---
 net/rfkill/core.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

Comments

kernel test robot Sept. 14, 2023, 6:47 p.m. UTC | #1 
Hi Johannes,

kernel test robot noticed the following build warnings:

[auto build test WARNING on wireless-next/main]
[also build test WARNING on wireless/main linus/master v6.6-rc1 next-20230914]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Johannes-Berg/rfkill-sync-before-userspace-visibility-changes/20230914-221103
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
patch link:    https://lore.kernel.org/r/20230914153604.9d1ce0f43ac8.If977317d8f6a0f557090defcd6aef67628f62ff7%40changeid
patch subject: [PATCH v2] rfkill: sync before userspace visibility/changes
config: i386-buildonly-randconfig-003-20230915 (https://download.01.org/0day-ci/archive/20230915/202309150223.e6DfafFk-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230915/202309150223.e6DfafFk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309150223.e6DfafFk-lkp@intel.com/

All warnings (new ones prefixed by >>):

   net/rfkill/core.c: In function 'soft_show':
   net/rfkill/core.c:735:2: error: implicit declaration of function 'rfkill_sync'; did you mean 'rfkill_event'? [-Werror=implicit-function-declaration]
     735 |  rfkill_sync(rfkill);
         |  ^~~~~~~~~~~
         |  rfkill_event
   net/rfkill/core.c: At top level:
>> net/rfkill/core.c:1044:13: warning: conflicting types for 'rfkill_sync'
    1044 | static void rfkill_sync(struct rfkill *rfkill)
         |             ^~~~~~~~~~~
   net/rfkill/core.c:1044:13: error: static declaration of 'rfkill_sync' follows non-static declaration
   net/rfkill/core.c:735:2: note: previous implicit declaration of 'rfkill_sync' was here
     735 |  rfkill_sync(rfkill);
         |  ^~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/rfkill_sync +1044 net/rfkill/core.c

  1043	
> 1044	static void rfkill_sync(struct rfkill *rfkill)
  1045	{
  1046		lockdep_assert_held(&rfkill_global_mutex);
  1047	
  1048		if (!rfkill->need_sync)
  1049			return;
  1050	
  1051		rfkill_set_block(rfkill, rfkill_global_states[rfkill->type].cur);
  1052		rfkill->need_sync = false;
  1053	}
  1054
kernel test robot Sept. 14, 2023, 11:34 p.m. UTC | #2 
Hi Johannes,

kernel test robot noticed the following build errors:

[auto build test ERROR on wireless-next/main]
[also build test ERROR on wireless/main linus/master v6.6-rc1 next-20230914]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Johannes-Berg/rfkill-sync-before-userspace-visibility-changes/20230914-221103
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
patch link:    https://lore.kernel.org/r/20230914153604.9d1ce0f43ac8.If977317d8f6a0f557090defcd6aef67628f62ff7%40changeid
patch subject: [PATCH v2] rfkill: sync before userspace visibility/changes
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20230915/202309150719.OIX0u8eK-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230915/202309150719.OIX0u8eK-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309150719.OIX0u8eK-lkp@intel.com/

All errors (new ones prefixed by >>):

>> net/rfkill/core.c:735:2: error: call to undeclared function 'rfkill_sync'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
           rfkill_sync(rfkill);
           ^
   net/rfkill/core.c:759:2: error: call to undeclared function 'rfkill_sync'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
           rfkill_sync(rfkill);
           ^
   net/rfkill/core.c:793:2: error: call to undeclared function 'rfkill_sync'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
           rfkill_sync(rfkill);
           ^
   net/rfkill/core.c:818:2: error: call to undeclared function 'rfkill_sync'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
           rfkill_sync(rfkill);
           ^
   net/rfkill/core.c:1044:13: error: static declaration of 'rfkill_sync' follows non-static declaration
   static void rfkill_sync(struct rfkill *rfkill)
               ^
   net/rfkill/core.c:735:2: note: previous implicit declaration is here
           rfkill_sync(rfkill);
           ^
   5 errors generated.


vim +/rfkill_sync +735 net/rfkill/core.c

   728	
   729	static ssize_t soft_show(struct device *dev, struct device_attribute *attr,
   730				 char *buf)
   731	{
   732		struct rfkill *rfkill = to_rfkill(dev);
   733	
   734		mutex_lock(&rfkill_global_mutex);
 > 735		rfkill_sync(rfkill);
   736		mutex_unlock(&rfkill_global_mutex);
   737	
   738		return sysfs_emit(buf, "%d\n", (rfkill->state & RFKILL_BLOCK_SW) ? 1 : 0);
   739	}
   740
diff mbox series

Patch

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 01fca7a10b4b..2768decb6cda 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -48,6 +48,7 @@  struct rfkill {
 	bool			persistent;
 	bool			polling_paused;
 	bool			suspended;
+	bool			need_sync;
 
 	const struct rfkill_ops	*ops;
 	void			*data;
@@ -730,6 +731,10 @@  static ssize_t soft_show(struct device *dev, struct device_attribute *attr,
 {
 	struct rfkill *rfkill = to_rfkill(dev);
 
+	mutex_lock(&rfkill_global_mutex);
+	rfkill_sync(rfkill);
+	mutex_unlock(&rfkill_global_mutex);
+
 	return sysfs_emit(buf, "%d\n", (rfkill->state & RFKILL_BLOCK_SW) ? 1 : 0);
 }
 
@@ -751,6 +756,7 @@  static ssize_t soft_store(struct device *dev, struct device_attribute *attr,
 		return -EINVAL;
 
 	mutex_lock(&rfkill_global_mutex);
+	rfkill_sync(rfkill);
 	rfkill_set_block(rfkill, state);
 	mutex_unlock(&rfkill_global_mutex);
 
@@ -783,6 +789,10 @@  static ssize_t state_show(struct device *dev, struct device_attribute *attr,
 {
 	struct rfkill *rfkill = to_rfkill(dev);
 
+	mutex_lock(&rfkill_global_mutex);
+	rfkill_sync(rfkill);
+	mutex_unlock(&rfkill_global_mutex);
+
 	return sysfs_emit(buf, "%d\n", user_state_from_blocked(rfkill->state));
 }
 
@@ -805,6 +815,7 @@  static ssize_t state_store(struct device *dev, struct device_attribute *attr,
 		return -EINVAL;
 
 	mutex_lock(&rfkill_global_mutex);
+	rfkill_sync(rfkill);
 	rfkill_set_block(rfkill, state == RFKILL_USER_STATE_SOFT_BLOCKED);
 	mutex_unlock(&rfkill_global_mutex);
 
@@ -1030,16 +1041,23 @@  static void rfkill_uevent_work(struct work_struct *work)
 	mutex_unlock(&rfkill_global_mutex);
 }
 
+static void rfkill_sync(struct rfkill *rfkill)
+{
+	lockdep_assert_held(&rfkill_global_mutex);
+
+	if (!rfkill->need_sync)
+		return;
+
+	rfkill_set_block(rfkill, rfkill_global_states[rfkill->type].cur);
+	rfkill->need_sync = false;
+}
+
 static void rfkill_sync_work(struct work_struct *work)
 {
-	struct rfkill *rfkill;
-	bool cur;
-
-	rfkill = container_of(work, struct rfkill, sync_work);
+	struct rfkill *rfkill = container_of(work, struct rfkill, sync_work);
 
 	mutex_lock(&rfkill_global_mutex);
-	cur = rfkill_global_states[rfkill->type].cur;
-	rfkill_set_block(rfkill, cur);
+	rfkill_sync(rfkill);
 	mutex_unlock(&rfkill_global_mutex);
 }
 
@@ -1087,6 +1105,7 @@  int __must_check rfkill_register(struct rfkill *rfkill)
 			round_jiffies_relative(POLL_INTERVAL));
 
 	if (!rfkill->persistent || rfkill_epo_lock_active) {
+		rfkill->need_sync = true;
 		schedule_work(&rfkill->sync_work);
 	} else {
 #ifdef CONFIG_RFKILL_INPUT
@@ -1171,6 +1190,7 @@  static int rfkill_fop_open(struct inode *inode, struct file *file)
 		ev = kzalloc(sizeof(*ev), GFP_KERNEL);
 		if (!ev)
 			goto free;
+		rfkill_sync(rfkill);
 		rfkill_fill_event(&ev->ev, rfkill, RFKILL_OP_ADD);
 		list_add_tail(&ev->list, &data->events);
 	}