BPF List
 help / color / mirror / Atom feed
* [bug report] bpf: Consolidate spin_lock, timer management into btf_record
@ 2022-11-15 13:06 Dan Carpenter
  2022-11-15 16:46 ` Kumar Kartikeya Dwivedi
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2022-11-15 13:06 UTC (permalink / raw)
  To: memxor; +Cc: bpf

[ Email screwup on my end means I have to resend two weeks of email.
  :/  -dan ]

Hello Kumar Kartikeya Dwivedi,

The patch db559117828d: "bpf: Consolidate spin_lock, timer management
into btf_record" from Nov 4, 2022, leads to the following Smatch
static checker warning:

	kernel/bpf/syscall.c:1002 map_check_btf()
	warn: ignoring unreachable code.

kernel/bpf/syscall.c
    946 static int map_check_btf(struct bpf_map *map, const struct btf *btf,
    947                          u32 btf_key_id, u32 btf_value_id)
    948 {
    949         const struct btf_type *key_type, *value_type;
    950         u32 key_size, value_size;
    951         int ret = 0;
    952 
    953         /* Some maps allow key to be unspecified. */
    954         if (btf_key_id) {
    955                 key_type = btf_type_id_size(btf, &btf_key_id, &key_size);
    956                 if (!key_type || key_size != map->key_size)
    957                         return -EINVAL;
    958         } else {
    959                 key_type = btf_type_by_id(btf, 0);
    960                 if (!map->ops->map_check_btf)
    961                         return -EINVAL;
    962         }
    963 
    964         value_type = btf_type_id_size(btf, &btf_value_id, &value_size);
    965         if (!value_type || value_size != map->value_size)
    966                 return -EINVAL;
    967 
    968         map->record = btf_parse_fields(btf, value_type, BPF_SPIN_LOCK | BPF_TIMER | BPF_KPTR,
    969                                        map->value_size);
    970         if (!IS_ERR_OR_NULL(map->record)) {
    971                 int i;
    972 
    973                 if (!bpf_capable()) {
    974                         ret = -EPERM;
    975                         goto free_map_tab;
    976                 }
    977                 if (map->map_flags & (BPF_F_RDONLY_PROG | BPF_F_WRONLY_PROG)) {
    978                         ret = -EACCES;
    979                         goto free_map_tab;
    980                 }
    981                 for (i = 0; i < sizeof(map->record->field_mask) * 8; i++) {
    982                         switch (map->record->field_mask & (1 << i)) {
    983                         case 0:
    984                                 continue;
    985                         case BPF_SPIN_LOCK:
    986                                 if (map->map_type != BPF_MAP_TYPE_HASH &&
    987                                     map->map_type != BPF_MAP_TYPE_ARRAY &&
    988                                     map->map_type != BPF_MAP_TYPE_CGROUP_STORAGE &&
    989                                     map->map_type != BPF_MAP_TYPE_SK_STORAGE &&
    990                                     map->map_type != BPF_MAP_TYPE_INODE_STORAGE &&
    991                                     map->map_type != BPF_MAP_TYPE_TASK_STORAGE &&
    992                                     map->map_type != BPF_MAP_TYPE_CGRP_STORAGE) {
    993                                         ret = -EOPNOTSUPP;
    994                                         goto free_map_tab;
    995                                 }
    996                                 break;
    997                         case BPF_TIMER:
    998                                 if (map->map_type != BPF_MAP_TYPE_HASH &&
    999                                     map->map_type != BPF_MAP_TYPE_LRU_HASH &&
    1000                                     map->map_type != BPF_MAP_TYPE_ARRAY) {
    1001                                         return -EOPNOTSUPP;
                                                 ^^^^^^^^^^^^^^^^^^^
--> 1002                                         goto free_map_tab;

s/return/ret =/

Surprised the coverity-bot hasn't complained about this...

    1003                                 }
    1004                                 break;
    1005                         case BPF_KPTR_UNREF:
    1006                         case BPF_KPTR_REF:
    1007                                 if (map->map_type != BPF_MAP_TYPE_HASH &&
    1008                                     map->map_type != BPF_MAP_TYPE_LRU_HASH &&
    1009                                     map->map_type != BPF_MAP_TYPE_ARRAY &&
    1010                                     map->map_type != BPF_MAP_TYPE_PERCPU_ARRAY) {
    1011                                         ret = -EOPNOTSUPP;
    1012                                         goto free_map_tab;
    1013                                 }
    1014                                 break;
    1015                         default:
    1016                                 /* Fail if map_type checks are missing for a field type */
    1017                                 ret = -EOPNOTSUPP;
    1018                                 goto free_map_tab;
    1019                         }
    1020                 }
    1021         }
    1022 
    1023         if (map->ops->map_check_btf) {
    1024                 ret = map->ops->map_check_btf(map, btf, key_type, value_type);
    1025                 if (ret < 0)
    1026                         goto free_map_tab;
    1027         }
    1028 
    1029         return ret;
    1030 free_map_tab:
    1031         bpf_map_free_record(map);
    1032         return ret;
    1033 }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-11-15 16:46 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-15 13:06 [bug report] bpf: Consolidate spin_lock, timer management into btf_record Dan Carpenter
2022-11-15 16:46 ` Kumar Kartikeya Dwivedi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox