[PATCH 2/4] kcsan, debugfs: refactor set_report_filterlist_whitelist() to return a value

ran xiaokai posted 4 patches 2 months ago
[PATCH 2/4] kcsan, debugfs: refactor set_report_filterlist_whitelist() to return a value
Posted by ran xiaokai 2 months ago
From: Ran Xiaokai <ran.xiaokai@zte.com.cn>

This is a preparation patch, when converted to rcu lock,
set_report_filterlist_whitelist() may fail due to memory alloction,
refactor it to return a value, so the error codes can be
passed to the userspace.

Signed-off-by: Ran Xiaokai <ran.xiaokai@zte.com.cn>
---
 kernel/kcsan/debugfs.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/kernel/kcsan/debugfs.c b/kernel/kcsan/debugfs.c
index ed483987869e..30547507f497 100644
--- a/kernel/kcsan/debugfs.c
+++ b/kernel/kcsan/debugfs.c
@@ -131,13 +131,14 @@ bool kcsan_skip_report_debugfs(unsigned long func_addr)
 	return ret;
 }
 
-static void set_report_filterlist_whitelist(bool whitelist)
+static ssize_t set_report_filterlist_whitelist(bool whitelist)
 {
 	unsigned long flags;
 
 	spin_lock_irqsave(&report_filterlist_lock, flags);
 	report_filterlist.whitelist = whitelist;
 	spin_unlock_irqrestore(&report_filterlist_lock, flags);
+	return 0;
 }
 
 /* Returns 0 on success, error-code otherwise. */
@@ -225,6 +226,7 @@ debugfs_write(struct file *file, const char __user *buf, size_t count, loff_t *o
 	char kbuf[KSYM_NAME_LEN];
 	char *arg;
 	const size_t read_len = min(count, sizeof(kbuf) - 1);
+	ssize_t ret;
 
 	if (copy_from_user(kbuf, buf, read_len))
 		return -EFAULT;
@@ -242,19 +244,19 @@ debugfs_write(struct file *file, const char __user *buf, size_t count, loff_t *o
 			return -EINVAL;
 		microbenchmark(iters);
 	} else if (!strcmp(arg, "whitelist")) {
-		set_report_filterlist_whitelist(true);
+		ret = set_report_filterlist_whitelist(true);
 	} else if (!strcmp(arg, "blacklist")) {
-		set_report_filterlist_whitelist(false);
+		ret = set_report_filterlist_whitelist(false);
 	} else if (arg[0] == '!') {
-		ssize_t ret = insert_report_filterlist(&arg[1]);
-
-		if (ret < 0)
-			return ret;
+		ret = insert_report_filterlist(&arg[1]);
 	} else {
 		return -EINVAL;
 	}
 
-	return count;
+	if (ret < 0)
+		return ret;
+	else
+		return count;
 }
 
 static const struct file_operations debugfs_ops =
-- 
2.15.2
Re: [PATCH 2/4] kcsan, debugfs: refactor set_report_filterlist_whitelist() to return a value
Posted by Marco Elver 1 month, 4 weeks ago
On Wed, 25 Sept 2024 at 16:32, ran xiaokai <ranxiaokai627@163.com> wrote:
>
> From: Ran Xiaokai <ran.xiaokai@zte.com.cn>
>
> This is a preparation patch, when converted to rcu lock,
> set_report_filterlist_whitelist() may fail due to memory alloction,
> refactor it to return a value, so the error codes can be
> passed to the userspace.
>
> Signed-off-by: Ran Xiaokai <ran.xiaokai@zte.com.cn>
> ---
>  kernel/kcsan/debugfs.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/kcsan/debugfs.c b/kernel/kcsan/debugfs.c
> index ed483987869e..30547507f497 100644
> --- a/kernel/kcsan/debugfs.c
> +++ b/kernel/kcsan/debugfs.c
> @@ -131,13 +131,14 @@ bool kcsan_skip_report_debugfs(unsigned long func_addr)
>         return ret;
>  }
>
> -static void set_report_filterlist_whitelist(bool whitelist)
> +static ssize_t set_report_filterlist_whitelist(bool whitelist)
>  {
>         unsigned long flags;
>
>         spin_lock_irqsave(&report_filterlist_lock, flags);
>         report_filterlist.whitelist = whitelist;
>         spin_unlock_irqrestore(&report_filterlist_lock, flags);
> +       return 0;
>  }
>
>  /* Returns 0 on success, error-code otherwise. */
> @@ -225,6 +226,7 @@ debugfs_write(struct file *file, const char __user *buf, size_t count, loff_t *o
>         char kbuf[KSYM_NAME_LEN];
>         char *arg;
>         const size_t read_len = min(count, sizeof(kbuf) - 1);
> +       ssize_t ret;

This may be uninitialized depending on the branch taken below.

>         if (copy_from_user(kbuf, buf, read_len))
>                 return -EFAULT;
> @@ -242,19 +244,19 @@ debugfs_write(struct file *file, const char __user *buf, size_t count, loff_t *o
>                         return -EINVAL;
>                 microbenchmark(iters);
>         } else if (!strcmp(arg, "whitelist")) {
> -               set_report_filterlist_whitelist(true);
> +               ret = set_report_filterlist_whitelist(true);
>         } else if (!strcmp(arg, "blacklist")) {
> -               set_report_filterlist_whitelist(false);
> +               ret = set_report_filterlist_whitelist(false);
>         } else if (arg[0] == '!') {
> -               ssize_t ret = insert_report_filterlist(&arg[1]);
> -
> -               if (ret < 0)
> -                       return ret;
> +               ret = insert_report_filterlist(&arg[1]);
>         } else {
>                 return -EINVAL;
>         }
>
> -       return count;
> +       if (ret < 0)
> +               return ret;
> +       else
> +               return count;
>  }
>
>  static const struct file_operations debugfs_ops =
> --
> 2.15.2
>
Re: [PATCH 2/4] kcsan, debugfs: refactor set_report_filterlist_whitelist() to return a value
Posted by kernel test robot 2 months ago
Hi ran,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on next-20240925]
[cannot apply to v6.11]
[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/ran-xiaokai/kcsan-debugfs-Remove-redundant-call-of-kallsyms_lookup_name/20240925-231034
base:   linus/master
patch link:    https://lore.kernel.org/r/20240925143154.2322926-3-ranxiaokai627%40163.com
patch subject: [PATCH 2/4] kcsan, debugfs: refactor set_report_filterlist_whitelist() to return a value
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20240926/202409261331.9NyGRPt2-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240926/202409261331.9NyGRPt2-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/202409261331.9NyGRPt2-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> kernel/kcsan/debugfs.c:243:7: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     243 |                 if (kstrtoul(&arg[strlen("microbench=")], 0, &iters))
         |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/kcsan/debugfs.c:256:6: note: uninitialized use occurs here
     256 |         if (ret < 0)
         |             ^~~
   kernel/kcsan/debugfs.c:243:3: note: remove the 'if' if its condition is always true
     243 |                 if (kstrtoul(&arg[strlen("microbench=")], 0, &iters))
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     244 |                         return -EINVAL;
>> kernel/kcsan/debugfs.c:238:13: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
     238 |         } else if (!strcmp(arg, "off")) {
         |                    ^~~~~~~~~~~~~~~~~~~
   kernel/kcsan/debugfs.c:256:6: note: uninitialized use occurs here
     256 |         if (ret < 0)
         |             ^~~
   kernel/kcsan/debugfs.c:238:9: note: remove the 'if' if its condition is always false
     238 |         } else if (!strcmp(arg, "off")) {
         |                ^~~~~~~~~~~~~~~~~~~~~~~~~~
     239 |                 WRITE_ONCE(kcsan_enabled, false);
         |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     240 |         } else if (str_has_prefix(arg, "microbench=")) {
         |         ~~~~~~
   kernel/kcsan/debugfs.c:236:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
     236 |         if (!strcmp(arg, "on")) {
         |             ^~~~~~~~~~~~~~~~~~
   kernel/kcsan/debugfs.c:256:6: note: uninitialized use occurs here
     256 |         if (ret < 0)
         |             ^~~
   kernel/kcsan/debugfs.c:236:2: note: remove the 'if' if its condition is always false
     236 |         if (!strcmp(arg, "on")) {
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~
     237 |                 WRITE_ONCE(kcsan_enabled, true);
         |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     238 |         } else if (!strcmp(arg, "off")) {
         |         ~~~~~~
   kernel/kcsan/debugfs.c:229:13: note: initialize the variable 'ret' to silence this warning
     229 |         ssize_t ret;
         |                    ^
         |                     = 0
   3 warnings generated.


vim +243 kernel/kcsan/debugfs.c

dfd402a4c4baae Marco Elver   2019-11-14  222  
5cbaefe9743bf1 Ingo Molnar   2019-11-20  223  static ssize_t
5cbaefe9743bf1 Ingo Molnar   2019-11-20  224  debugfs_write(struct file *file, const char __user *buf, size_t count, loff_t *off)
dfd402a4c4baae Marco Elver   2019-11-14  225  {
dfd402a4c4baae Marco Elver   2019-11-14  226  	char kbuf[KSYM_NAME_LEN];
dfd402a4c4baae Marco Elver   2019-11-14  227  	char *arg;
43d631bf06ec96 Thorsten Blum 2024-06-24  228  	const size_t read_len = min(count, sizeof(kbuf) - 1);
52313281c8b7ca Ran Xiaokai   2024-09-25  229  	ssize_t ret;
dfd402a4c4baae Marco Elver   2019-11-14  230  
dfd402a4c4baae Marco Elver   2019-11-14  231  	if (copy_from_user(kbuf, buf, read_len))
dfd402a4c4baae Marco Elver   2019-11-14  232  		return -EFAULT;
dfd402a4c4baae Marco Elver   2019-11-14  233  	kbuf[read_len] = '\0';
dfd402a4c4baae Marco Elver   2019-11-14  234  	arg = strstrip(kbuf);
dfd402a4c4baae Marco Elver   2019-11-14  235  
dfd402a4c4baae Marco Elver   2019-11-14  236  	if (!strcmp(arg, "on")) {
dfd402a4c4baae Marco Elver   2019-11-14  237  		WRITE_ONCE(kcsan_enabled, true);
dfd402a4c4baae Marco Elver   2019-11-14 @238  	} else if (!strcmp(arg, "off")) {
dfd402a4c4baae Marco Elver   2019-11-14  239  		WRITE_ONCE(kcsan_enabled, false);
a4e74fa5f0d3e2 Marco Elver   2020-07-31  240  	} else if (str_has_prefix(arg, "microbench=")) {
dfd402a4c4baae Marco Elver   2019-11-14  241  		unsigned long iters;
dfd402a4c4baae Marco Elver   2019-11-14  242  
a4e74fa5f0d3e2 Marco Elver   2020-07-31 @243  		if (kstrtoul(&arg[strlen("microbench=")], 0, &iters))
dfd402a4c4baae Marco Elver   2019-11-14  244  			return -EINVAL;
dfd402a4c4baae Marco Elver   2019-11-14  245  		microbenchmark(iters);
dfd402a4c4baae Marco Elver   2019-11-14  246  	} else if (!strcmp(arg, "whitelist")) {
52313281c8b7ca Ran Xiaokai   2024-09-25  247  		ret = set_report_filterlist_whitelist(true);
dfd402a4c4baae Marco Elver   2019-11-14  248  	} else if (!strcmp(arg, "blacklist")) {
52313281c8b7ca Ran Xiaokai   2024-09-25  249  		ret = set_report_filterlist_whitelist(false);
dfd402a4c4baae Marco Elver   2019-11-14  250  	} else if (arg[0] == '!') {
52313281c8b7ca Ran Xiaokai   2024-09-25  251  		ret = insert_report_filterlist(&arg[1]);
dfd402a4c4baae Marco Elver   2019-11-14  252  	} else {
dfd402a4c4baae Marco Elver   2019-11-14  253  		return -EINVAL;
dfd402a4c4baae Marco Elver   2019-11-14  254  	}
dfd402a4c4baae Marco Elver   2019-11-14  255  
52313281c8b7ca Ran Xiaokai   2024-09-25  256  	if (ret < 0)
52313281c8b7ca Ran Xiaokai   2024-09-25  257  		return ret;
52313281c8b7ca Ran Xiaokai   2024-09-25  258  	else
dfd402a4c4baae Marco Elver   2019-11-14  259  		return count;
dfd402a4c4baae Marco Elver   2019-11-14  260  }
dfd402a4c4baae Marco Elver   2019-11-14  261  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki