| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c0560b16985daf420d35bec6f4957414fff44bd2.camel@ibm.com>
Date: Fri, 6 Feb 2026 20:44:25 +0000
From: Viacheslav Dubeyko <Slava.Dubeyko@....com>
To: "idryomov@...il.com" <idryomov@...il.com>,
Alex Markuze
<amarkuze@...hat.com>,
"dan.carpenter@...aro.org" <dan.carpenter@...aro.org>
CC: "ceph-devel@...r.kernel.org" <ceph-devel@...r.kernel.org>,
"sage@...tank.com" <sage@...tank.com>,
"linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
Subject: Re: [bug report] crush: remove forcefeed functionality
On Fri, 2026-02-06 at 16:39 +0300, Dan Carpenter wrote:
> [ Smatch checking is paused while we raise funding. #SadFace
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_aTaiGSbWZ9DJaGo7-40stanley.mountain_&d=DwIBAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=EbbQA8mLawUrIpoBP1JgkEbj9ykB2zMAgU-BpxccK9crlqQp8eHphKm2eDfswppo&s=4dnJgIrt1z5jJRZwXTmcMBeS0RZ5lg-CZ04H1P9fcrE&e= -dan ]
>
> Hello Ceph Maintainers,
>
> Commit 41ebcc0907c5 ("crush: remove forcefeed functionality") from
> May 7, 2012 (linux-next), leads to the following Smatch static
> checker warning:
>
> net/ceph/crush/mapper.c:1015 crush_do_rule()
> warn: iterator 'j' not incremented
Yeah, it looks like an issue.
>
> net/ceph/crush/mapper.c
> 897 int crush_do_rule(const struct crush_map *map,
> 898 int ruleno, int x, int *result, int result_max,
> 899 const __u32 *weight, int weight_max,
> 900 void *cwin, const struct crush_choose_arg *choose_args)
> 901 {
> 902 int result_len;
> 903 struct crush_work *cw = cwin;
> 904 int *a = cwin + map->working_size;
> 905 int *b = a + result_max;
> 906 int *c = b + result_max;
> 907 int *w = a;
> 908 int *o = b;
> 909 int recurse_to_leaf;
> 910 int wsize = 0;
> 911 int osize;
> 912 const struct crush_rule *rule;
> 913 __u32 step;
> 914 int i, j;
> 915 int numrep;
> 916 int out_size;
> 917 /*
> 918 * the original choose_total_tries value was off by one (it
> 919 * counted "retries" and not "tries"). add one.
> 920 */
> 921 int choose_tries = map->choose_total_tries + 1;
> 922 int choose_leaf_tries = 0;
> 923 /*
> 924 * the local tries values were counted as "retries", though,
> 925 * and need no adjustment
> 926 */
> 927 int choose_local_retries = map->choose_local_tries;
> 928 int choose_local_fallback_retries = map->choose_local_fallback_tries;
> 929
> 930 int vary_r = map->chooseleaf_vary_r;
> 931 int stable = map->chooseleaf_stable;
> 932
> 933 if ((__u32)ruleno >= map->max_rules) {
> 934 dprintk(" bad ruleno %d\n", ruleno);
> 935 return 0;
> 936 }
> 937
> 938 rule = map->rules[ruleno];
> 939 result_len = 0;
> 940
> 941 for (step = 0; step < rule->len; step++) {
> 942 int firstn = 0;
> 943 const struct crush_rule_step *curstep = &rule->steps[step];
> 944
> 945 switch (curstep->op) {
> 946 case CRUSH_RULE_TAKE:
> 947 if ((curstep->arg1 >= 0 &&
> 948 curstep->arg1 < map->max_devices) ||
> 949 (-1-curstep->arg1 >= 0 &&
> 950 -1-curstep->arg1 < map->max_buckets &&
> 951 map->buckets[-1-curstep->arg1])) {
> 952 w[0] = curstep->arg1;
> 953 wsize = 1;
> 954 } else {
> 955 dprintk(" bad take value %d\n", curstep->arg1);
> 956 }
> 957 break;
> 958
> 959 case CRUSH_RULE_SET_CHOOSE_TRIES:
> 960 if (curstep->arg1 > 0)
> 961 choose_tries = curstep->arg1;
> 962 break;
> 963
> 964 case CRUSH_RULE_SET_CHOOSELEAF_TRIES:
> 965 if (curstep->arg1 > 0)
> 966 choose_leaf_tries = curstep->arg1;
> 967 break;
> 968
> 969 case CRUSH_RULE_SET_CHOOSE_LOCAL_TRIES:
> 970 if (curstep->arg1 >= 0)
> 971 choose_local_retries = curstep->arg1;
> 972 break;
> 973
> 974 case CRUSH_RULE_SET_CHOOSE_LOCAL_FALLBACK_TRIES:
> 975 if (curstep->arg1 >= 0)
> 976 choose_local_fallback_retries = curstep->arg1;
> 977 break;
> 978
> 979 case CRUSH_RULE_SET_CHOOSELEAF_VARY_R:
> 980 if (curstep->arg1 >= 0)
> 981 vary_r = curstep->arg1;
> 982 break;
> 983
> 984 case CRUSH_RULE_SET_CHOOSELEAF_STABLE:
> 985 if (curstep->arg1 >= 0)
> 986 stable = curstep->arg1;
> 987 break;
> 988
> 989 case CRUSH_RULE_CHOOSELEAF_FIRSTN:
> 990 case CRUSH_RULE_CHOOSE_FIRSTN:
> 991 firstn = 1;
> 992 fallthrough;
> 993 case CRUSH_RULE_CHOOSELEAF_INDEP:
> 994 case CRUSH_RULE_CHOOSE_INDEP:
> 995 if (wsize == 0)
> 996 break;
> 997
> 998 recurse_to_leaf =
> 999 curstep->op ==
> 1000 CRUSH_RULE_CHOOSELEAF_FIRSTN ||
> 1001 curstep->op ==
> 1002 CRUSH_RULE_CHOOSELEAF_INDEP;
> 1003
> 1004 /* reset output */
> 1005 osize = 0;
> 1006
> 1007 for (i = 0; i < wsize; i++) {
> 1008 int bno;
> 1009 numrep = curstep->arg1;
> 1010 if (numrep <= 0) {
> 1011 numrep += result_max;
> 1012 if (numrep <= 0)
> 1013 continue;
> 1014 }
> --> 1015 j = 0;
> ^^^^^
It looks like intentional initialization of variable. But let me spend some time
to better understand the crush_choose_firstn() and crush_choose_indep() logic
and the history of this function modifications in commits.
Thanks,
Slava.
>
> 1016 /* make sure bucket id is valid */
> 1017 bno = -1 - w[i];
> 1018 if (bno < 0 || bno >= map->max_buckets) {
> 1019 /* w[i] is probably CRUSH_ITEM_NONE */
> 1020 dprintk(" bad w[i] %d\n", w[i]);
> 1021 continue;
> 1022 }
> 1023 if (firstn) {
> 1024 int recurse_tries;
> 1025 if (choose_leaf_tries)
> 1026 recurse_tries =
> 1027 choose_leaf_tries;
> 1028 else if (map->chooseleaf_descend_once)
> 1029 recurse_tries = 1;
> 1030 else
> 1031 recurse_tries = choose_tries;
> 1032 osize += crush_choose_firstn(
> 1033 map,
> 1034 cw,
> 1035 map->buckets[bno],
> 1036 weight, weight_max,
> 1037 x, numrep,
> 1038 curstep->arg2,
> 1039 o+osize, j,
> 1040 result_max-osize,
> 1041 choose_tries,
> 1042 recurse_tries,
> 1043 choose_local_retries,
> 1044 choose_local_fallback_retries,
> 1045 recurse_to_leaf,
> 1046 vary_r,
> 1047 stable,
> 1048 c+osize,
> 1049 0,
> 1050 choose_args);
> 1051 } else {
> 1052 out_size = ((numrep < (result_max-osize)) ?
> 1053 numrep : (result_max-osize));
> 1054 crush_choose_indep(
> 1055 map,
> 1056 cw,
> 1057 map->buckets[bno],
> 1058 weight, weight_max,
> 1059 x, out_size, numrep,
> 1060 curstep->arg2,
> 1061 o+osize, j,
> 1062 choose_tries,
> 1063 choose_leaf_tries ?
> 1064 choose_leaf_tries : 1,
> 1065 recurse_to_leaf,
> 1066 c+osize,
> 1067 0,
> 1068 choose_args);
> 1069 osize += out_size;
> 1070 }
>
> There used to be a j++ around here but it was deleted.
>
> 1071 }
> 1072
> 1073 if (recurse_to_leaf)
> 1074 /* copy final _leaf_ values to output set */
> 1075 memcpy(o, c, osize*sizeof(*o));
> 1076
> 1077 /* swap o and w arrays */
> 1078 swap(o, w);
> 1079 wsize = osize;
> 1080 break;
> 1081
> 1082
> 1083 case CRUSH_RULE_EMIT:
> 1084 for (i = 0; i < wsize && result_len < result_max; i++) {
> 1085 result[result_len] = w[i];
> 1086 result_len++;
> 1087 }
> 1088 wsize = 0;
> 1089 break;
> 1090
> 1091 default:
> 1092 dprintk(" unknown op %d at step %d\n",
> 1093 curstep->op, step);
> 1094 break;
> 1095 }
> 1096 }
> 1097
> 1098 return result_len;
> 1099 }
>
> regards,
> dan carpenter
Powered by blists - more mailing lists