This email adjusts the code format.
I don't understand why we don't check path for NULL, bpf_link__pin is an
external
interface, It will be called by external functions and provide input
parameters,
for example in samples/bpf/hbm.c:
201 link = bpf_program__attach_cgroup(bpf_prog, cg1);
202 if (libbpf_get_error(link)) {
203 fprintf(stderr, "ERROR: bpf_program__attach_cgroup
failed\n");
204 goto err;
205 }
206
207 sprintf(cg_pin_path, "/sys/fs/bpf/hbm%d", cg_id);
208 rc = bpf_link__pin(link, cg_pin_path);
209 if (rc < 0) {
210 printf("ERROR: bpf_link__pin failed: %d\n", rc);
211 goto err;
212 }
if cg_pin_path is NULL, strdup(NULL) will trigger a segmentation fault in
make_parent_dir, I think we should avoid this and add null-pointer checking
for path, just like check_path:
7673 static int check_path(const char *path)
7674 {
7675 char *cp, errmsg[STRERR_BUFSIZE];
7676 struct statfs st_fs;
7677 char *dname, *dir;
7678 int err = 0;
7679
7680 if (path == NULL)
7681 return -EINVAL;
7682
7683 dname = strdup(path);
7684 if (dname == NULL)
7685 return -ENOMEM;
7686
7687 dir = dirname(dname);
7688 if (statfs(dir, &st_fs)) {
7689 cp = libbpf_strerror_r(errno, errmsg,
sizeof(errmsg));
7690 pr_warn("failed to statfs %s: %s\n", dir, cp);
7691 err = -errno;
7692 }
7693 free(dname);
7694
7695 if (!err && st_fs.f_type != BPF_FS_MAGIC) {
7696 pr_warn("specified path %s is not on BPF FS\n",
path);
7697 err = -EINVAL;
7698 }
7699
7700 return err;
7701 }
Thanks.
在 2022/4/22 0:55, Andrii Nakryiko 写道:
On Thu, Apr 21, 2022 at 6:01 AM Gaosheng Cui <cuigaosheng1@xxxxxxxxxx> wrote:
The make_parent_dir is called without null-pointer checking for path,
such as bpf_link__pin. To ensure there is no null-pointer dereference
in make_parent_dir, so make_parent_dir requires additional null-pointer
checking for path.
Signed-off-by: Gaosheng Cui <cuigaosheng1@xxxxxxxxxx>
---
tools/lib/bpf/libbpf.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b53e51884f9e..5786e6184bf5 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -7634,6 +7634,9 @@ static int make_parent_dir(const char *path)
char *dname, *dir;
int err = 0;
+ if (path == NULL)
+ return -EINVAL;
+
API contract is that path shouldn't be NULL. Just like we don't check
link, obj, prog for NULL in every single API, I don't think we need to
do it here, unless I'm missing something?
dname = strdup(path);
if (dname == NULL)
return -ENOMEM;
--
2.25.1
.
