All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <error27@gmail.com>
To: memxor@gmail.com
Cc: bpf@vger.kernel.org
Subject: [bug report] bpf: Consolidate spin_lock, timer management into btf_record
Date: Tue, 15 Nov 2022 16:06:25 +0300	[thread overview]
Message-ID: <Y3OO0aZps7WeVpFA@kili> (raw)

[ 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

             reply	other threads:[~2022-11-15 13:06 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-15 13:06 Dan Carpenter [this message]
2022-11-15 16:46 ` [bug report] bpf: Consolidate spin_lock, timer management into btf_record Kumar Kartikeya Dwivedi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y3OO0aZps7WeVpFA@kili \
    --to=error27@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=memxor@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.