Commit 28513fccf0ceefb8171ddc0cefa429b82e92a2c9

Authored by Milan Broz
Committed by Alasdair G Kergon
1 parent 7e507eb643

dm crypt: simplify crypt_config destruction logic

Use just one label and reuse common destructor for crypt target.

Parse remaining argv arguments in logic order.

Also do not ignore error values from IV init and set key functions.

No functional change in this patch except changed return codes
based on above.

Signed-off-by: Milan Broz <mbroz@redhat.com>
Signed-off-by: Alasdair G Kergon <agk@redhat.com>

Showing 1 changed file with 93 additions and 87 deletions Side-by-side Diff

drivers/md/dm-crypt.c
... ... @@ -999,6 +999,45 @@
999 999 return crypto_ablkcipher_setkey(cc->tfm, cc->key, cc->key_size);
1000 1000 }
1001 1001  
  1002 +static void crypt_dtr(struct dm_target *ti)
  1003 +{
  1004 + struct crypt_config *cc = ti->private;
  1005 +
  1006 + ti->private = NULL;
  1007 +
  1008 + if (!cc)
  1009 + return;
  1010 +
  1011 + if (cc->io_queue)
  1012 + destroy_workqueue(cc->io_queue);
  1013 + if (cc->crypt_queue)
  1014 + destroy_workqueue(cc->crypt_queue);
  1015 +
  1016 + if (cc->bs)
  1017 + bioset_free(cc->bs);
  1018 +
  1019 + if (cc->page_pool)
  1020 + mempool_destroy(cc->page_pool);
  1021 + if (cc->req_pool)
  1022 + mempool_destroy(cc->req_pool);
  1023 + if (cc->io_pool)
  1024 + mempool_destroy(cc->io_pool);
  1025 +
  1026 + if (cc->iv_gen_ops && cc->iv_gen_ops->dtr)
  1027 + cc->iv_gen_ops->dtr(cc);
  1028 +
  1029 + if (cc->tfm && !IS_ERR(cc->tfm))
  1030 + crypto_free_ablkcipher(cc->tfm);
  1031 +
  1032 + if (cc->dev)
  1033 + dm_put_device(ti, cc->dev);
  1034 +
  1035 + kfree(cc->iv_mode);
  1036 +
  1037 + /* Must zero key material before freeing */
  1038 + kzfree(cc);
  1039 +}
  1040 +
1002 1041 /*
1003 1042 * Construct an encryption mapping:
1004 1043 * <cipher> <key> <iv_offset> <dev_path> <start>
... ... @@ -1006,7 +1045,6 @@
1006 1045 static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
1007 1046 {
1008 1047 struct crypt_config *cc;
1009   - struct crypto_ablkcipher *tfm;
1010 1048 char *tmp;
1011 1049 char *cipher;
1012 1050 char *chainmode;
... ... @@ -1014,6 +1052,7 @@
1014 1052 char *ivopts;
1015 1053 unsigned int key_size;
1016 1054 unsigned long long tmpll;
  1055 + int ret = -EINVAL;
1017 1056  
1018 1057 if (argc != 5) {
1019 1058 ti->error = "Not enough arguments";
1020 1059  
... ... @@ -1032,12 +1071,13 @@
1032 1071 key_size = strlen(argv[1]) >> 1;
1033 1072  
1034 1073 cc = kzalloc(sizeof(*cc) + key_size * sizeof(u8), GFP_KERNEL);
1035   - if (cc == NULL) {
1036   - ti->error =
1037   - "Cannot allocate transparent encryption context";
  1074 + if (!cc) {
  1075 + ti->error = "Cannot allocate transparent encryption context";
1038 1076 return -ENOMEM;
1039 1077 }
1040 1078  
  1079 + ti->private = cc;
  1080 +
1041 1081 /* Compatibility mode for old dm-crypt cipher strings */
1042 1082 if (!chainmode || (strcmp(chainmode, "plain") == 0 && !ivmode)) {
1043 1083 chainmode = "cbc";
1044 1084  
1045 1085  
1046 1086  
1047 1087  
1048 1088  
1049 1089  
1050 1090  
1051 1091  
... ... @@ -1046,35 +1086,36 @@
1046 1086  
1047 1087 if (strcmp(chainmode, "ecb") && !ivmode) {
1048 1088 ti->error = "This chaining mode requires an IV mechanism";
1049   - goto bad_cipher;
  1089 + goto bad;
1050 1090 }
1051 1091  
  1092 + ret = -ENOMEM;
1052 1093 if (snprintf(cc->cipher, CRYPTO_MAX_ALG_NAME, "%s(%s)",
1053 1094 chainmode, cipher) >= CRYPTO_MAX_ALG_NAME) {
1054 1095 ti->error = "Chain mode + cipher name is too long";
1055   - goto bad_cipher;
  1096 + goto bad;
1056 1097 }
1057 1098  
1058   - tfm = crypto_alloc_ablkcipher(cc->cipher, 0, 0);
1059   - if (IS_ERR(tfm)) {
  1099 + cc->tfm = crypto_alloc_ablkcipher(cc->cipher, 0, 0);
  1100 + if (IS_ERR(cc->tfm)) {
1060 1101 ti->error = "Error allocating crypto tfm";
1061   - goto bad_cipher;
  1102 + goto bad;
1062 1103 }
1063 1104  
1064 1105 strcpy(cc->cipher, cipher);
1065 1106 strcpy(cc->chainmode, chainmode);
1066   - cc->tfm = tfm;
1067 1107  
1068   - if (crypt_set_key(cc, argv[1]) < 0) {
  1108 + ret = crypt_set_key(cc, argv[1]);
  1109 + if (ret < 0) {
1069 1110 ti->error = "Error decoding and setting key";
1070   - goto bad_ivmode;
  1111 + goto bad;
1071 1112 }
1072 1113  
1073 1114 /*
1074 1115 * Choose ivmode. Valid modes: "plain", "essiv:<esshash>", "benbi".
1075 1116 * See comments at iv code
1076 1117 */
1077   -
  1118 + ret = -EINVAL;
1078 1119 if (ivmode == NULL)
1079 1120 cc->iv_gen_ops = NULL;
1080 1121 else if (strcmp(ivmode, "plain") == 0)
1081 1122  
1082 1123  
1083 1124  
... ... @@ -1089,20 +1130,28 @@
1089 1130 cc->iv_gen_ops = &crypt_iv_null_ops;
1090 1131 else {
1091 1132 ti->error = "Invalid IV mode";
1092   - goto bad_ivmode;
  1133 + goto bad;
1093 1134 }
1094 1135  
1095   - if (cc->iv_gen_ops && cc->iv_gen_ops->ctr &&
1096   - cc->iv_gen_ops->ctr(cc, ti, ivopts) < 0)
1097   - goto bad_ivmode;
  1136 + /* Allocate IV */
  1137 + if (cc->iv_gen_ops && cc->iv_gen_ops->ctr) {
  1138 + ret = cc->iv_gen_ops->ctr(cc, ti, ivopts);
  1139 + if (ret < 0) {
  1140 + ti->error = "Error creating IV";
  1141 + goto bad;
  1142 + }
  1143 + }
1098 1144  
1099   - if (cc->iv_gen_ops && cc->iv_gen_ops->init &&
1100   - cc->iv_gen_ops->init(cc) < 0) {
1101   - ti->error = "Error initialising IV";
1102   - goto bad_slab_pool;
  1145 + /* Initialize IV (set keys for ESSIV etc) */
  1146 + if (cc->iv_gen_ops && cc->iv_gen_ops->init) {
  1147 + ret = cc->iv_gen_ops->init(cc);
  1148 + if (ret < 0) {
  1149 + ti->error = "Error initialising IV";
  1150 + goto bad;
  1151 + }
1103 1152 }
1104 1153  
1105   - cc->iv_size = crypto_ablkcipher_ivsize(tfm);
  1154 + cc->iv_size = crypto_ablkcipher_ivsize(cc->tfm);
1106 1155 if (cc->iv_size)
1107 1156 /* at least a 64 bit sector number should fit in our buffer */
1108 1157 cc->iv_size = max(cc->iv_size,
1109 1158  
1110 1159  
1111 1160  
1112 1161  
1113 1162  
1114 1163  
1115 1164  
1116 1165  
1117 1166  
1118 1167  
1119 1168  
1120 1169  
... ... @@ -1116,62 +1165,65 @@
1116 1165 }
1117 1166 }
1118 1167  
  1168 + ret = -ENOMEM;
1119 1169 cc->io_pool = mempool_create_slab_pool(MIN_IOS, _crypt_io_pool);
1120 1170 if (!cc->io_pool) {
1121 1171 ti->error = "Cannot allocate crypt io mempool";
1122   - goto bad_slab_pool;
  1172 + goto bad;
1123 1173 }
1124 1174  
1125 1175 cc->dmreq_start = sizeof(struct ablkcipher_request);
1126   - cc->dmreq_start += crypto_ablkcipher_reqsize(tfm);
  1176 + cc->dmreq_start += crypto_ablkcipher_reqsize(cc->tfm);
1127 1177 cc->dmreq_start = ALIGN(cc->dmreq_start, crypto_tfm_ctx_alignment());
1128   - cc->dmreq_start += crypto_ablkcipher_alignmask(tfm) &
  1178 + cc->dmreq_start += crypto_ablkcipher_alignmask(cc->tfm) &
1129 1179 ~(crypto_tfm_ctx_alignment() - 1);
1130 1180  
1131 1181 cc->req_pool = mempool_create_kmalloc_pool(MIN_IOS, cc->dmreq_start +
1132 1182 sizeof(struct dm_crypt_request) + cc->iv_size);
1133 1183 if (!cc->req_pool) {
1134 1184 ti->error = "Cannot allocate crypt request mempool";
1135   - goto bad_req_pool;
  1185 + goto bad;
1136 1186 }
1137 1187 cc->req = NULL;
1138 1188  
1139 1189 cc->page_pool = mempool_create_page_pool(MIN_POOL_PAGES, 0);
1140 1190 if (!cc->page_pool) {
1141 1191 ti->error = "Cannot allocate page mempool";
1142   - goto bad_page_pool;
  1192 + goto bad;
1143 1193 }
1144 1194  
1145 1195 cc->bs = bioset_create(MIN_IOS, 0);
1146 1196 if (!cc->bs) {
1147 1197 ti->error = "Cannot allocate crypt bioset";
1148   - goto bad_bs;
  1198 + goto bad;
1149 1199 }
1150 1200  
  1201 + ret = -EINVAL;
1151 1202 if (sscanf(argv[2], "%llu", &tmpll) != 1) {
1152 1203 ti->error = "Invalid iv_offset sector";
1153   - goto bad_device;
  1204 + goto bad;
1154 1205 }
1155 1206 cc->iv_offset = tmpll;
1156 1207  
  1208 + if (dm_get_device(ti, argv[3], dm_table_get_mode(ti->table), &cc->dev)) {
  1209 + ti->error = "Device lookup failed";
  1210 + goto bad;
  1211 + }
  1212 +
1157 1213 if (sscanf(argv[4], "%llu", &tmpll) != 1) {
1158 1214 ti->error = "Invalid device sector";
1159   - goto bad_device;
  1215 + goto bad;
1160 1216 }
1161 1217 cc->start = tmpll;
1162 1218  
1163   - if (dm_get_device(ti, argv[3], dm_table_get_mode(ti->table), &cc->dev)) {
1164   - ti->error = "Device lookup failed";
1165   - goto bad_device;
1166   - }
1167   -
  1219 + ret = -ENOMEM;
1168 1220 if (ivmode && cc->iv_gen_ops) {
1169 1221 if (ivopts)
1170 1222 *(ivopts - 1) = ':';
1171 1223 cc->iv_mode = kstrdup(ivmode, GFP_KERNEL);
1172 1224 if (!cc->iv_mode) {
1173 1225 ti->error = "Error kmallocing iv_mode string";
1174   - goto bad_ivmode_string;
  1226 + goto bad;
1175 1227 }
1176 1228 } else
1177 1229 cc->iv_mode = NULL;
1178 1230  
1179 1231  
1180 1232  
... ... @@ -1179,67 +1231,21 @@
1179 1231 cc->io_queue = create_singlethread_workqueue("kcryptd_io");
1180 1232 if (!cc->io_queue) {
1181 1233 ti->error = "Couldn't create kcryptd io queue";
1182   - goto bad_io_queue;
  1234 + goto bad;
1183 1235 }
1184 1236  
1185 1237 cc->crypt_queue = create_singlethread_workqueue("kcryptd");
1186 1238 if (!cc->crypt_queue) {
1187 1239 ti->error = "Couldn't create kcryptd queue";
1188   - goto bad_crypt_queue;
  1240 + goto bad;
1189 1241 }
1190 1242  
1191 1243 ti->num_flush_requests = 1;
1192   - ti->private = cc;
1193 1244 return 0;
1194 1245  
1195   -bad_crypt_queue:
1196   - destroy_workqueue(cc->io_queue);
1197   -bad_io_queue:
1198   - kfree(cc->iv_mode);
1199   -bad_ivmode_string:
1200   - dm_put_device(ti, cc->dev);
1201   -bad_device:
1202   - bioset_free(cc->bs);
1203   -bad_bs:
1204   - mempool_destroy(cc->page_pool);
1205   -bad_page_pool:
1206   - mempool_destroy(cc->req_pool);
1207   -bad_req_pool:
1208   - mempool_destroy(cc->io_pool);
1209   -bad_slab_pool:
1210   - if (cc->iv_gen_ops && cc->iv_gen_ops->dtr)
1211   - cc->iv_gen_ops->dtr(cc);
1212   -bad_ivmode:
1213   - crypto_free_ablkcipher(tfm);
1214   -bad_cipher:
1215   - /* Must zero key material before freeing */
1216   - kzfree(cc);
1217   - return -EINVAL;
1218   -}
1219   -
1220   -static void crypt_dtr(struct dm_target *ti)
1221   -{
1222   - struct crypt_config *cc = (struct crypt_config *) ti->private;
1223   -
1224   - destroy_workqueue(cc->io_queue);
1225   - destroy_workqueue(cc->crypt_queue);
1226   -
1227   - if (cc->req)
1228   - mempool_free(cc->req, cc->req_pool);
1229   -
1230   - bioset_free(cc->bs);
1231   - mempool_destroy(cc->page_pool);
1232   - mempool_destroy(cc->req_pool);
1233   - mempool_destroy(cc->io_pool);
1234   -
1235   - kfree(cc->iv_mode);
1236   - if (cc->iv_gen_ops && cc->iv_gen_ops->dtr)
1237   - cc->iv_gen_ops->dtr(cc);
1238   - crypto_free_ablkcipher(cc->tfm);
1239   - dm_put_device(ti, cc->dev);
1240   -
1241   - /* Must zero key material before freeing */
1242   - kzfree(cc);
  1246 +bad:
  1247 + crypt_dtr(ti);
  1248 + return ret;
1243 1249 }
1244 1250  
1245 1251 static int crypt_map(struct dm_target *ti, struct bio *bio,