| Message ID | 20240628084857.1719108-4-bigeasy@linutronix.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: sparse cleanup. | expand |
Hi Sebastian, kernel test robot noticed the following build errors: [auto build test ERROR on next-20240628] [cannot apply to bpf-next/master bpf/master v6.10-rc5 v6.10-rc4 v6.10-rc3 linus/master v6.10-rc5] [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/Sebastian-Andrzej-Siewior/bpf-Add-casts-to-keep-sparse-quiet/20240629-142618 base: next-20240628 patch link: https://lore.kernel.org/r/20240628084857.1719108-4-bigeasy%40linutronix.de patch subject: [PATCH bpf-next 3/3] bpf: Implement bpf_check_basics_ok() as a macro. config: i386-buildonly-randconfig-004-20240701 compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1) reproduce (this is a W=1 build): 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/202407010432.E7ktzEgq-lkp@intel.com/ All errors (new ones prefixed by >>): >> net/core/filter.c:1379:7: error: use of undeclared identifier '__ret'; did you mean '__ret_'? 1379 | if (!bpf_check_basics_ok(fprog->filter, fprog->len)) | ^ net/core/filter.c:1047:3: note: expanded from macro 'bpf_check_basics_ok' 1047 | __ret = false; \ | ^ net/core/filter.c:1379:7: note: '__ret_' declared here net/core/filter.c:1043:7: note: expanded from macro 'bpf_check_basics_ok' 1043 | bool __ret_ = true; \ | ^ >> net/core/filter.c:1379:7: error: use of undeclared identifier '__ret'; did you mean '__ret_'? 1379 | if (!bpf_check_basics_ok(fprog->filter, fprog->len)) | ^ net/core/filter.c:1049:3: note: expanded from macro 'bpf_check_basics_ok' 1049 | __ret = false; \ | ^ net/core/filter.c:1379:7: note: '__ret_' declared here net/core/filter.c:1043:7: note: expanded from macro 'bpf_check_basics_ok' 1043 | bool __ret_ = true; \ | ^ >> net/core/filter.c:1379:7: error: use of undeclared identifier '__ret'; did you mean '__ret_'? 1379 | if (!bpf_check_basics_ok(fprog->filter, fprog->len)) | ^ net/core/filter.c:1050:2: note: expanded from macro 'bpf_check_basics_ok' 1050 | __ret; \ | ^ net/core/filter.c:1379:7: note: '__ret_' declared here net/core/filter.c:1043:7: note: expanded from macro 'bpf_check_basics_ok' 1043 | bool __ret_ = true; \ | ^ net/core/filter.c:1426:7: error: use of undeclared identifier '__ret'; did you mean '__ret_'? 1426 | if (!bpf_check_basics_ok(fprog->filter, fprog->len)) | ^ net/core/filter.c:1047:3: note: expanded from macro 'bpf_check_basics_ok' 1047 | __ret = false; \ | ^ net/core/filter.c:1426:7: note: '__ret_' declared here net/core/filter.c:1043:7: note: expanded from macro 'bpf_check_basics_ok' 1043 | bool __ret_ = true; \ | ^ net/core/filter.c:1426:7: error: use of undeclared identifier '__ret'; did you mean '__ret_'? 1426 | if (!bpf_check_basics_ok(fprog->filter, fprog->len)) | ^ net/core/filter.c:1049:3: note: expanded from macro 'bpf_check_basics_ok' 1049 | __ret = false; \ | ^ net/core/filter.c:1426:7: note: '__ret_' declared here net/core/filter.c:1043:7: note: expanded from macro 'bpf_check_basics_ok' 1043 | bool __ret_ = true; \ | ^ net/core/filter.c:1426:7: error: use of undeclared identifier '__ret'; did you mean '__ret_'? 1426 | if (!bpf_check_basics_ok(fprog->filter, fprog->len)) | ^ net/core/filter.c:1050:2: note: expanded from macro 'bpf_check_basics_ok' 1050 | __ret; \ | ^ net/core/filter.c:1426:7: note: '__ret_' declared here net/core/filter.c:1043:7: note: expanded from macro 'bpf_check_basics_ok' 1043 | bool __ret_ = true; \ | ^ net/core/filter.c:1504:7: error: use of undeclared identifier '__ret'; did you mean '__ret_'? 1504 | if (!bpf_check_basics_ok(fprog->filter, fprog->len)) | ^ net/core/filter.c:1047:3: note: expanded from macro 'bpf_check_basics_ok' 1047 | __ret = false; \ | ^ net/core/filter.c:1504:7: note: '__ret_' declared here net/core/filter.c:1043:7: note: expanded from macro 'bpf_check_basics_ok' 1043 | bool __ret_ = true; \ | ^ net/core/filter.c:1504:7: error: use of undeclared identifier '__ret'; did you mean '__ret_'? 1504 | if (!bpf_check_basics_ok(fprog->filter, fprog->len)) | ^ net/core/filter.c:1049:3: note: expanded from macro 'bpf_check_basics_ok' 1049 | __ret = false; \ | ^ net/core/filter.c:1504:7: note: '__ret_' declared here net/core/filter.c:1043:7: note: expanded from macro 'bpf_check_basics_ok' 1043 | bool __ret_ = true; \ | ^ net/core/filter.c:1504:7: error: use of undeclared identifier '__ret'; did you mean '__ret_'? 1504 | if (!bpf_check_basics_ok(fprog->filter, fprog->len)) | ^ net/core/filter.c:1050:2: note: expanded from macro 'bpf_check_basics_ok' 1050 | __ret; \ | ^ net/core/filter.c:1504:7: note: '__ret_' declared here net/core/filter.c:1043:7: note: expanded from macro 'bpf_check_basics_ok' 1043 | bool __ret_ = true; \ | ^ 9 errors generated. vim +1379 net/core/filter.c 302d663740cfaf Jiri Pirko 2012-03-31 1362 302d663740cfaf Jiri Pirko 2012-03-31 1363 /** 7ae457c1e5b45a Alexei Starovoitov 2014-07-30 1364 * bpf_prog_create - create an unattached filter c6c4b97c6b7003 Randy Dunlap 2012-06-08 1365 * @pfp: the unattached filter that is created 677a9fd3e6e6e0 Tobias Klauser 2014-06-24 1366 * @fprog: the filter program 302d663740cfaf Jiri Pirko 2012-03-31 1367 * c6c4b97c6b7003 Randy Dunlap 2012-06-08 1368 * Create a filter independent of any socket. We first run some 302d663740cfaf Jiri Pirko 2012-03-31 1369 * sanity checks on it to make sure it does not explode on us later. 302d663740cfaf Jiri Pirko 2012-03-31 1370 * If an error occurs or there is insufficient memory for the filter 302d663740cfaf Jiri Pirko 2012-03-31 1371 * a negative errno code is returned. On success the return is zero. 302d663740cfaf Jiri Pirko 2012-03-31 1372 */ 7ae457c1e5b45a Alexei Starovoitov 2014-07-30 1373 int bpf_prog_create(struct bpf_prog **pfp, struct sock_fprog_kern *fprog) 302d663740cfaf Jiri Pirko 2012-03-31 1374 { 009937e78a4555 Alexei Starovoitov 2014-07-30 1375 unsigned int fsize = bpf_classic_proglen(fprog); 7ae457c1e5b45a Alexei Starovoitov 2014-07-30 1376 struct bpf_prog *fp; 302d663740cfaf Jiri Pirko 2012-03-31 1377 302d663740cfaf Jiri Pirko 2012-03-31 1378 /* Make sure new filter is there and in the right amounts. */ f7bd9e36ee4a4c Daniel Borkmann 2016-06-10 @1379 if (!bpf_check_basics_ok(fprog->filter, fprog->len)) 302d663740cfaf Jiri Pirko 2012-03-31 1380 return -EINVAL; 302d663740cfaf Jiri Pirko 2012-03-31 1381 60a3b2253c413c Daniel Borkmann 2014-09-02 1382 fp = bpf_prog_alloc(bpf_prog_size(fprog->len), 0); 302d663740cfaf Jiri Pirko 2012-03-31 1383 if (!fp) 302d663740cfaf Jiri Pirko 2012-03-31 1384 return -ENOMEM; a3ea269b8bcdbb Daniel Borkmann 2014-03-28 1385 302d663740cfaf Jiri Pirko 2012-03-31 1386 memcpy(fp->insns, fprog->filter, fsize); 302d663740cfaf Jiri Pirko 2012-03-31 1387 302d663740cfaf Jiri Pirko 2012-03-31 1388 fp->len = fprog->len; a3ea269b8bcdbb Daniel Borkmann 2014-03-28 1389 /* Since unattached filters are not copied back to user a3ea269b8bcdbb Daniel Borkmann 2014-03-28 1390 * space through sk_get_filter(), we do not need to hold a3ea269b8bcdbb Daniel Borkmann 2014-03-28 1391 * a copy here, and can spare us the work. a3ea269b8bcdbb Daniel Borkmann 2014-03-28 1392 */ a3ea269b8bcdbb Daniel Borkmann 2014-03-28 1393 fp->orig_prog = NULL; 302d663740cfaf Jiri Pirko 2012-03-31 1394 7ae457c1e5b45a Alexei Starovoitov 2014-07-30 1395 /* bpf_prepare_filter() already takes care of freeing bd4cf0ed331a27 Alexei Starovoitov 2014-03-28 1396 * memory in case something goes wrong. bd4cf0ed331a27 Alexei Starovoitov 2014-03-28 1397 */ 4ae92bc77ac8e6 Nicolas Schichan 2015-05-06 1398 fp = bpf_prepare_filter(fp, NULL); bd4cf0ed331a27 Alexei Starovoitov 2014-03-28 1399 if (IS_ERR(fp)) bd4cf0ed331a27 Alexei Starovoitov 2014-03-28 1400 return PTR_ERR(fp); 302d663740cfaf Jiri Pirko 2012-03-31 1401 302d663740cfaf Jiri Pirko 2012-03-31 1402 *pfp = fp; 302d663740cfaf Jiri Pirko 2012-03-31 1403 return 0; 302d663740cfaf Jiri Pirko 2012-03-31 1404 } 7ae457c1e5b45a Alexei Starovoitov 2014-07-30 1405 EXPORT_SYMBOL_GPL(bpf_prog_create); 302d663740cfaf Jiri Pirko 2012-03-31 1406
Hi Sebastian, kernel test robot noticed the following build errors: [auto build test ERROR on next-20240628] [cannot apply to bpf-next/master bpf/master v6.10-rc5 v6.10-rc4 v6.10-rc3 linus/master v6.10-rc5] [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/Sebastian-Andrzej-Siewior/bpf-Add-casts-to-keep-sparse-quiet/20240629-142618 base: next-20240628 patch link: https://lore.kernel.org/r/20240628084857.1719108-4-bigeasy%40linutronix.de patch subject: [PATCH bpf-next 3/3] bpf: Implement bpf_check_basics_ok() as a macro. config: i386-buildonly-randconfig-006-20240701 compiler: gcc-9 (Ubuntu 9.5.0-4ubuntu2) 9.5.0 reproduce (this is a W=1 build): 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/202407010544.dILQ6Mb0-lkp@intel.com/ All error/warnings (new ones prefixed by >>): net/core/filter.c: In function 'bpf_prog_create': >> net/core/filter.c:1047:3: error: '__ret' undeclared (first use in this function); did you mean '__ret_'? 1047 | __ret = false; \ | ^~~~~ net/core/filter.c:1379:7: note: in expansion of macro 'bpf_check_basics_ok' 1379 | if (!bpf_check_basics_ok(fprog->filter, fprog->len)) | ^~~~~~~~~~~~~~~~~~~ net/core/filter.c:1047:3: note: each undeclared identifier is reported only once for each function it appears in 1047 | __ret = false; \ | ^~~~~ net/core/filter.c:1379:7: note: in expansion of macro 'bpf_check_basics_ok' 1379 | if (!bpf_check_basics_ok(fprog->filter, fprog->len)) | ^~~~~~~~~~~~~~~~~~~ >> net/core/filter.c:1043:7: warning: unused variable '__ret_' [-Wunused-variable] 1043 | bool __ret_ = true; \ | ^~~~~~ net/core/filter.c:1379:7: note: in expansion of macro 'bpf_check_basics_ok' 1379 | if (!bpf_check_basics_ok(fprog->filter, fprog->len)) | ^~~~~~~~~~~~~~~~~~~ net/core/filter.c: In function 'bpf_prog_create_from_user': >> net/core/filter.c:1047:3: error: '__ret' undeclared (first use in this function); did you mean '__ret_'? 1047 | __ret = false; \ | ^~~~~ net/core/filter.c:1426:7: note: in expansion of macro 'bpf_check_basics_ok' 1426 | if (!bpf_check_basics_ok(fprog->filter, fprog->len)) | ^~~~~~~~~~~~~~~~~~~ >> net/core/filter.c:1043:7: warning: unused variable '__ret_' [-Wunused-variable] 1043 | bool __ret_ = true; \ | ^~~~~~ net/core/filter.c:1426:7: note: in expansion of macro 'bpf_check_basics_ok' 1426 | if (!bpf_check_basics_ok(fprog->filter, fprog->len)) | ^~~~~~~~~~~~~~~~~~~ net/core/filter.c: In function '__get_filter': >> net/core/filter.c:1047:3: error: '__ret' undeclared (first use in this function); did you mean '__ret_'? 1047 | __ret = false; \ | ^~~~~ net/core/filter.c:1504:7: note: in expansion of macro 'bpf_check_basics_ok' 1504 | if (!bpf_check_basics_ok(fprog->filter, fprog->len)) | ^~~~~~~~~~~~~~~~~~~ >> net/core/filter.c:1043:7: warning: unused variable '__ret_' [-Wunused-variable] 1043 | bool __ret_ = true; \ | ^~~~~~ net/core/filter.c:1504:7: note: in expansion of macro 'bpf_check_basics_ok' 1504 | if (!bpf_check_basics_ok(fprog->filter, fprog->len)) | ^~~~~~~~~~~~~~~~~~~ vim +1047 net/core/filter.c 1037 1038 /* macro instead of a function to avoid woring about _filter which might be a 1039 * user or kernel pointer. It does not matter for the NULL check. 1040 */ 1041 #define bpf_check_basics_ok(fprog_filter, fprog_flen) \ 1042 ({ \ > 1043 bool __ret_ = true; \ 1044 u16 __flen = fprog_flen; \ 1045 \ 1046 if (!(fprog_filter)) \ > 1047 __ret = false; \ 1048 else if (__flen == 0 || __flen > BPF_MAXINSNS) \ 1049 __ret = false; \ 1050 __ret; \ 1051 }) 1052
diff --git a/net/core/filter.c b/net/core/filter.c index 11939971f3c6a..72ccce80f9f15 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -1035,16 +1035,20 @@ static bool chk_code_allowed(u16 code_to_probe) return codes[code_to_probe]; } -static bool bpf_check_basics_ok(const struct sock_filter *filter, - unsigned int flen) -{ - if (filter == NULL) - return false; - if (flen == 0 || flen > BPF_MAXINSNS) - return false; - - return true; -} + /* macro instead of a function to avoid woring about _filter which might be a + * user or kernel pointer. It does not matter for the NULL check. + */ +#define bpf_check_basics_ok(fprog_filter, fprog_flen) \ +({ \ + bool __ret_ = true; \ + u16 __flen = fprog_flen; \ + \ + if (!(fprog_filter)) \ + __ret = false; \ + else if (__flen == 0 || __flen > BPF_MAXINSNS) \ + __ret = false; \ + __ret; \ +}) /** * bpf_check_classic - verify socket filter code
sparse complains about the argument type for filter that is passed to bpf_check_basics_ok(). There are two users of the function where the variable is with __user attribute one without. The pointer is only checked against NULL so there is no access to the content and so no need for any user-wrapper. Adding the __user to the declaration doesn't solve anything because there is one kernel user so it will be wrong again. Splitting the function in two seems an overkill because the function is small and simple. Make a macro based on the function which does not trigger a sparse warning. The change to a macro and "unsigned int" -> "u16" for `flen' alters gcc's code generation a bit. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- net/core/filter.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)