[bpf-next] selftests/bpf: Fix strncpy() fortify warning

Message ID tencent_ED443B037FA1D72CB4CB38958E57F2D12007@qq.com
State New
Headers
Series [bpf-next] selftests/bpf: Fix strncpy() fortify warning |

Commit Message

Rong Tao Nov. 1, 2022, 12:21 p.m. UTC
  From: Rong Tao <rongtao@cestc.cn>

move libbpf_strlcpy() to bpf_util.h, and replace strncpy() with
libbpf_strlcpy(), fix compile warning.

Compile samples/bpf, warning:
$ cd samples/bpf
$ make
...
cgroup_helpers.c: In function ‘__enable_controllers’:
cgroup_helpers.c:80:17: warning: ‘strncpy’ specified bound 4097 equals destination size [-Wstringop-truncation]
   80 |                 strncpy(enable, controllers, sizeof(enable));
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Rong Tao <rongtao@cestc.cn>
---
 tools/testing/selftests/bpf/bpf_util.h       | 19 +++++++++++++++++++
 tools/testing/selftests/bpf/cgroup_helpers.c |  3 ++-
 tools/testing/selftests/bpf/xsk.c            | 19 -------------------
 3 files changed, 21 insertions(+), 20 deletions(-)
  

Comments

Daniel Borkmann Nov. 1, 2022, 1:56 p.m. UTC | #1
On 11/1/22 1:21 PM, Rong Tao wrote:
> From: Rong Tao <rongtao@cestc.cn>
> 
> move libbpf_strlcpy() to bpf_util.h, and replace strncpy() with
> libbpf_strlcpy(), fix compile warning.
> 
> Compile samples/bpf, warning:
> $ cd samples/bpf
> $ make
> ...
> cgroup_helpers.c: In function ‘__enable_controllers’:
> cgroup_helpers.c:80:17: warning: ‘strncpy’ specified bound 4097 equals destination size [-Wstringop-truncation]
>     80 |                 strncpy(enable, controllers, sizeof(enable));
>        |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 

BPF CI now throws a new warning after your patch:

https://github.com/kernel-patches/bpf/actions/runs/3369416153/jobs/5589052613

   [...]
   xsk.c:536:2: error: call to undeclared function 'libbpf_strlcpy'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
           libbpf_strlcpy(ifr.ifr_name, ctx->ifname, IFNAMSIZ);
           ^
   xsk.c:755:2: error: call to undeclared function 'libbpf_strlcpy'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
           libbpf_strlcpy(ctx->ifname, ifname, IFNAMSIZ);
           ^
   xsk.c:942:2: error: call to undeclared function 'libbpf_strlcpy'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
           libbpf_strlcpy(ctx->ifname, ifname, IFNAMSIZ);
           ^
   3 errors generated.
   make: *** [Makefile:166: /tmp/work/bpf/bpf/tools/testing/selftests/bpf/xsk.o] Error 1
   make: *** Waiting for unfinished jobs....
  

Patch

diff --git a/tools/testing/selftests/bpf/bpf_util.h b/tools/testing/selftests/bpf/bpf_util.h
index a3352a64c067..bf78212ff6e9 100644
--- a/tools/testing/selftests/bpf/bpf_util.h
+++ b/tools/testing/selftests/bpf/bpf_util.h
@@ -20,6 +20,25 @@  static inline unsigned int bpf_num_possible_cpus(void)
 	return possible_cpus;
 }
 
+/* Copy up to sz - 1 bytes from zero-terminated src string and ensure that dst
+ * is zero-terminated string no matter what (unless sz == 0, in which case
+ * it's a no-op). It's conceptually close to FreeBSD's strlcpy(), but differs
+ * in what is returned. Given this is internal helper, it's trivial to extend
+ * this, when necessary. Use this instead of strncpy inside libbpf source code.
+ */
+static inline void libbpf_strlcpy(char *dst, const char *src, size_t sz)
+{
+	size_t i;
+
+	if (sz == 0)
+		return;
+
+	sz--;
+	for (i = 0; i < sz && src[i]; i++)
+		dst[i] = src[i];
+	dst[i] = '\0';
+}
+
 #define __bpf_percpu_val_align	__attribute__((__aligned__(8)))
 
 #define BPF_DECLARE_PERCPU(type, name)				\
diff --git a/tools/testing/selftests/bpf/cgroup_helpers.c b/tools/testing/selftests/bpf/cgroup_helpers.c
index e914cc45b766..e33b70e509da 100644
--- a/tools/testing/selftests/bpf/cgroup_helpers.c
+++ b/tools/testing/selftests/bpf/cgroup_helpers.c
@@ -13,6 +13,7 @@ 
 #include <ftw.h>
 
 #include "cgroup_helpers.h"
+#include "bpf_util.h"
 
 /*
  * To avoid relying on the system setup, when setup_cgroup_env is called
@@ -77,7 +78,7 @@  static int __enable_controllers(const char *cgroup_path, const char *controllers
 		enable[len] = 0;
 		close(fd);
 	} else {
-		strncpy(enable, controllers, sizeof(enable));
+		libbpf_strlcpy(enable, controllers, sizeof(enable));
 	}
 
 	snprintf(path, sizeof(path), "%s/cgroup.subtree_control", cgroup_path);
diff --git a/tools/testing/selftests/bpf/xsk.c b/tools/testing/selftests/bpf/xsk.c
index 0b3ff49c740d..4b6890e2a0a9 100644
--- a/tools/testing/selftests/bpf/xsk.c
+++ b/tools/testing/selftests/bpf/xsk.c
@@ -521,25 +521,6 @@  static int xsk_create_bpf_link(struct xsk_socket *xsk)
 	return 0;
 }
 
-/* Copy up to sz - 1 bytes from zero-terminated src string and ensure that dst
- * is zero-terminated string no matter what (unless sz == 0, in which case
- * it's a no-op). It's conceptually close to FreeBSD's strlcpy(), but differs
- * in what is returned. Given this is internal helper, it's trivial to extend
- * this, when necessary. Use this instead of strncpy inside libbpf source code.
- */
-static inline void libbpf_strlcpy(char *dst, const char *src, size_t sz)
-{
-        size_t i;
-
-        if (sz == 0)
-                return;
-
-        sz--;
-        for (i = 0; i < sz && src[i]; i++)
-                dst[i] = src[i];
-        dst[i] = '\0';
-}
-
 static int xsk_get_max_queues(struct xsk_socket *xsk)
 {
 	struct ethtool_channels channels = { .cmd = ETHTOOL_GCHANNELS };