* [PATCHv2 4/5] block/sed: Embed function data into the function sequence
From: Jon Derrick @ 2017-02-18 0:00 UTC (permalink / raw)
Cc: Jon Derrick, linux-block, linux-nvme, Scott Bauer,
Rafael Antognolli, Jens Axboe, Christoph Hellwig
In-Reply-To: <1487376029-22662-1-git-send-email-jonathan.derrick@intel.com>
By embedding the function data with the function sequence, this
eliminates a lot of the code that needs to set it in other places.
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
block/sed-opal.c | 358 +++++++++++++++++++++----------------------------------
1 file changed, 138 insertions(+), 220 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 4fc4d7b..e2e3228 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -34,7 +34,11 @@
#define IO_BUFFER_LENGTH 2048
#define MAX_TOKS 64
-typedef int (*opal_step)(struct opal_dev *dev);
+typedef struct opal_step {
+ int (*fn)(struct opal_dev *dev, void *data);
+ void *data;
+} opal_step;
+typedef int (cont_fn)(struct opal_dev *dev);
enum opal_atom_width {
OPAL_WIDTH_TINY,
@@ -80,8 +84,7 @@ struct opal_dev {
void *data;
sec_send_recv *send_recv;
- const opal_step *funcs;
- void **func_data;
+ opal_step *funcs;
int state;
struct mutex dev_lock;
u16 comid;
@@ -213,8 +216,6 @@ struct opal_dev {
{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x08, 0x03 },
};
-typedef int (cont_fn)(struct opal_dev *dev);
-
static int end_opal_session_error(struct opal_dev *dev);
struct opal_suspend_data {
@@ -375,15 +376,15 @@ static void check_geometry(struct opal_dev *dev, const void *data)
static int next(struct opal_dev *dev)
{
- opal_step func;
+ opal_step *func;
int error = 0;
do {
- func = dev->funcs[dev->state];
- if (!func)
+ func = &dev->funcs[dev->state];
+ if (!func->fn)
break;
- error = func(dev);
+ error = func->fn(dev, func->data);
if (error) {
pr_err("Error on step function: %d with error %d: %s\n",
dev->state, error,
@@ -483,7 +484,7 @@ static int opal_discovery0_end(struct opal_dev *dev)
return 0;
}
-static int opal_discovery0(struct opal_dev *dev)
+static int opal_discovery0(struct opal_dev *dev, void *data)
{
int ret;
@@ -1018,7 +1019,7 @@ static int finalize_and_send(struct opal_dev *dev, cont_fn cont)
return opal_send_recv(dev, cont);
}
-static int gen_key(struct opal_dev *dev)
+static int gen_key(struct opal_dev *dev, void *data)
{
const u8 *method;
u8 uid[OPAL_UID_LENGTH];
@@ -1072,15 +1073,14 @@ static int get_active_key_cont(struct opal_dev *dev)
return 0;
}
-static int get_active_key(struct opal_dev *dev)
+static int get_active_key(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
int err = 0;
- u8 *lr;
+ u8 *lr = data;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
- lr = dev->func_data[dev->state];
err = build_locking_range(uid, sizeof(uid), *lr);
if (err)
@@ -1163,17 +1163,16 @@ static inline int enable_global_lr(struct opal_dev *dev, u8 *uid,
return err;
}
-static int setup_locking_range(struct opal_dev *dev)
+static int setup_locking_range(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
- struct opal_user_lr_setup *setup;
+ struct opal_user_lr_setup *setup = data;
u8 lr;
int err = 0;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
- setup = dev->func_data[dev->state];
lr = setup->session.opal_key.lr;
err = build_locking_range(uid, sizeof(uid), lr);
if (err)
@@ -1286,20 +1285,19 @@ static int start_generic_opal_session(struct opal_dev *dev,
return finalize_and_send(dev, start_opal_session_cont);
}
-static int start_anybodyASP_opal_session(struct opal_dev *dev)
+static int start_anybodyASP_opal_session(struct opal_dev *dev, void *data)
{
return start_generic_opal_session(dev, OPAL_ANYBODY_UID,
OPAL_ADMINSP_UID, NULL, 0);
}
-static int start_SIDASP_opal_session(struct opal_dev *dev)
+static int start_SIDASP_opal_session(struct opal_dev *dev, void *data)
{
int ret;
const u8 *key = dev->prev_data;
- struct opal_key *okey;
if (!key) {
- okey = dev->func_data[dev->state];
+ const struct opal_key *okey = data;
ret = start_generic_opal_session(dev, OPAL_SID_UID,
OPAL_ADMINSP_UID,
okey->key,
@@ -1314,22 +1312,21 @@ static int start_SIDASP_opal_session(struct opal_dev *dev)
return ret;
}
-static inline int start_admin1LSP_opal_session(struct opal_dev *dev)
+static inline int start_admin1LSP_opal_session(struct opal_dev *dev, void *data)
{
- struct opal_key *key = dev->func_data[dev->state];
-
+ struct opal_key *key = data;
return start_generic_opal_session(dev, OPAL_ADMIN1_UID,
OPAL_LOCKINGSP_UID,
key->key, key->key_len);
}
-static int start_auth_opal_session(struct opal_dev *dev)
+static int start_auth_opal_session(struct opal_dev *dev, void *data)
{
+ struct opal_session_info *session = data;
u8 lk_ul_user[OPAL_UID_LENGTH];
+ size_t keylen = session->opal_key.key_len;
int err = 0;
- struct opal_session_info *session = dev->func_data[dev->state];
- size_t keylen = session->opal_key.key_len;
u8 *key = session->opal_key.key;
u32 hsn = GENERIC_HOST_SESSION_NUM;
@@ -1379,7 +1376,7 @@ static int start_auth_opal_session(struct opal_dev *dev)
return finalize_and_send(dev, start_opal_session_cont);
}
-static int revert_tper(struct opal_dev *dev)
+static int revert_tper(struct opal_dev *dev, void *data)
{
int err = 0;
@@ -1401,9 +1398,9 @@ static int revert_tper(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int internal_activate_user(struct opal_dev *dev)
+static int internal_activate_user(struct opal_dev *dev, void *data)
{
- struct opal_session_info *session = dev->func_data[dev->state];
+ struct opal_session_info *session = data;
u8 uid[OPAL_UID_LENGTH];
int err = 0;
@@ -1436,15 +1433,14 @@ static int internal_activate_user(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int erase_locking_range(struct opal_dev *dev)
+static int erase_locking_range(struct opal_dev *dev, void *data)
{
- struct opal_session_info *session;
+ struct opal_session_info *session = data;
u8 uid[OPAL_UID_LENGTH];
int err = 0;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
- session = dev->func_data[dev->state];
if (build_locking_range(uid, sizeof(uid), session->opal_key.lr) < 0)
return -ERANGE;
@@ -1463,9 +1459,9 @@ static int erase_locking_range(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int set_mbr_done(struct opal_dev *dev)
+static int set_mbr_done(struct opal_dev *dev, void *data)
{
- u8 mbr_done_tf = *(u8 *)dev->func_data[dev->state];
+ u8 mbr_done_tf = *(u8 *)data;
int err = 0;
clear_opal_cmd(dev);
@@ -1495,9 +1491,9 @@ static int set_mbr_done(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int set_mbr_enable_disable(struct opal_dev *dev)
+static int set_mbr_enable_disable(struct opal_dev *dev, void *data)
{
- u8 mbr_en_dis = *(u8 *)dev->func_data[dev->state];
+ u8 mbr_en_dis = *(u8 *)data;
int err = 0;
clear_opal_cmd(dev);
@@ -1554,11 +1550,10 @@ static int generic_pw_cmd(u8 *key, size_t key_len, u8 *cpin_uid,
return err;
}
-static int set_new_pw(struct opal_dev *dev)
+static int set_new_pw(struct opal_dev *dev, void *data)
{
u8 cpin_uid[OPAL_UID_LENGTH];
- struct opal_session_info *usr = dev->func_data[dev->state];
-
+ struct opal_session_info *usr = data;
memcpy(cpin_uid, opaluid[OPAL_C_PIN_ADMIN1], OPAL_UID_LENGTH);
@@ -1579,10 +1574,10 @@ static int set_new_pw(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int set_sid_cpin_pin(struct opal_dev *dev)
+static int set_sid_cpin_pin(struct opal_dev *dev, void *data)
{
u8 cpin_uid[OPAL_UID_LENGTH];
- struct opal_key *key = dev->func_data[dev->state];
+ struct opal_key *key = data;
memcpy(cpin_uid, opaluid[OPAL_C_PIN_SID], OPAL_UID_LENGTH);
@@ -1593,18 +1588,16 @@ static int set_sid_cpin_pin(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int add_user_to_lr(struct opal_dev *dev)
+static int add_user_to_lr(struct opal_dev *dev, void *data)
{
u8 lr_buffer[OPAL_UID_LENGTH];
u8 user_uid[OPAL_UID_LENGTH];
- struct opal_lock_unlock *lkul;
+ struct opal_lock_unlock *lkul = data;
int err = 0;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
- lkul = dev->func_data[dev->state];
-
memcpy(lr_buffer, opaluid[OPAL_LOCKINGRANGE_ACE_RDLOCKED],
OPAL_UID_LENGTH);
@@ -1671,11 +1664,11 @@ static int add_user_to_lr(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int lock_unlock_locking_range(struct opal_dev *dev)
+static int lock_unlock_locking_range(struct opal_dev *dev, void *data)
{
u8 lr_buffer[OPAL_UID_LENGTH];
const u8 *method;
- struct opal_lock_unlock *lkul;
+ struct opal_lock_unlock *lkul = data;
u8 read_locked = 1, write_locked = 1;
int err = 0;
@@ -1683,7 +1676,6 @@ static int lock_unlock_locking_range(struct opal_dev *dev)
set_comid(dev, dev->comid);
method = opalmethod[OPAL_SET];
- lkul = dev->func_data[dev->state];
if (build_locking_range(lr_buffer, sizeof(lr_buffer),
lkul->session.opal_key.lr) < 0)
return -ERANGE;
@@ -1735,19 +1727,18 @@ static int lock_unlock_locking_range(struct opal_dev *dev)
}
-static int lock_unlock_locking_range_sum(struct opal_dev *dev)
+static int lock_unlock_locking_range_sum(struct opal_dev *dev, void *data)
{
u8 lr_buffer[OPAL_UID_LENGTH];
u8 read_locked = 1, write_locked = 1;
const u8 *method;
- struct opal_lock_unlock *lkul;
+ struct opal_lock_unlock *lkul = data;
int ret;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
method = opalmethod[OPAL_SET];
- lkul = dev->func_data[dev->state];
if (build_locking_range(lr_buffer, sizeof(lr_buffer),
lkul->session.opal_key.lr) < 0)
return -ERANGE;
@@ -1778,9 +1769,9 @@ static int lock_unlock_locking_range_sum(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int activate_lsp(struct opal_dev *dev)
+static int activate_lsp(struct opal_dev *dev, void *data)
{
- struct opal_lr_act *opal_act;
+ struct opal_lr_act *opal_act = data;
u8 user_lr[OPAL_UID_LENGTH];
u8 uint_3 = 0x83;
int err = 0, i;
@@ -1788,8 +1779,6 @@ static int activate_lsp(struct opal_dev *dev)
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
- opal_act = dev->func_data[dev->state];
-
add_token_u8(&err, dev, OPAL_CALL);
add_token_bytestring(&err, dev, opaluid[OPAL_LOCKINGSP_UID],
OPAL_UID_LENGTH);
@@ -1854,7 +1843,7 @@ static int get_lsp_lifecycle_cont(struct opal_dev *dev)
}
/* Determine if we're in the Manufactured Inactive or Active state */
-static int get_lsp_lifecycle(struct opal_dev *dev)
+static int get_lsp_lifecycle(struct opal_dev *dev, void *data)
{
int err = 0;
@@ -1915,14 +1904,13 @@ static int get_msid_cpin_pin_cont(struct opal_dev *dev)
return 0;
}
-static int get_msid_cpin_pin(struct opal_dev *dev)
+static int get_msid_cpin_pin(struct opal_dev *dev, void *data)
{
int err = 0;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
-
add_token_u8(&err, dev, OPAL_CALL);
add_token_bytestring(&err, dev, opaluid[OPAL_C_PIN_MSID],
OPAL_UID_LENGTH);
@@ -1963,7 +1951,7 @@ static int build_end_opal_session(struct opal_dev *dev)
return err;
}
-static int end_opal_session(struct opal_dev *dev)
+static int end_opal_session(struct opal_dev *dev, void *data)
{
int ret = build_end_opal_session(dev);
@@ -1974,9 +1962,9 @@ static int end_opal_session(struct opal_dev *dev)
static int end_opal_session_error(struct opal_dev *dev)
{
- const opal_step error_end_session[] = {
- end_opal_session,
- NULL,
+ opal_step error_end_session[] = {
+ { end_opal_session, },
+ { NULL, }
};
dev->funcs = error_end_session;
dev->state = 0;
@@ -1984,21 +1972,20 @@ static int end_opal_session_error(struct opal_dev *dev)
}
static inline void setup_opal_dev(struct opal_dev *dev,
- const opal_step *funcs)
+ opal_step *funcs)
{
dev->state = 0;
dev->funcs = funcs;
dev->tsn = 0;
dev->hsn = 0;
- dev->func_data = NULL;
dev->prev_data = NULL;
}
static int check_opal_support(struct opal_dev *dev)
{
- static const opal_step funcs[] = {
- opal_discovery0,
- NULL
+ opal_step funcs[] = {
+ { opal_discovery0, },
+ { NULL, }
};
int ret;
@@ -2034,24 +2021,18 @@ struct opal_dev *init_opal_dev(void *data, sec_send_recv *send_recv)
static int opal_secure_erase_locking_range(struct opal_dev *dev,
struct opal_session_info *opal_session)
{
- void *data[3] = { NULL };
- static const opal_step erase_funcs[] = {
- opal_discovery0,
- start_auth_opal_session,
- get_active_key,
- gen_key,
- end_opal_session,
- NULL,
+ opal_step erase_funcs[] = {
+ { opal_discovery0, },
+ { start_auth_opal_session, opal_session },
+ { get_active_key, &opal_session->opal_key.lr },
+ { gen_key, },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, erase_funcs);
-
- dev->func_data = data;
- dev->func_data[1] = opal_session;
- dev->func_data[2] = &opal_session->opal_key.lr;
-
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2060,23 +2041,17 @@ static int opal_secure_erase_locking_range(struct opal_dev *dev,
static int opal_erase_locking_range(struct opal_dev *dev,
struct opal_session_info *opal_session)
{
- void *data[3] = { NULL };
- static const opal_step erase_funcs[] = {
- opal_discovery0,
- start_auth_opal_session,
- erase_locking_range,
- end_opal_session,
- NULL,
+ opal_step erase_funcs[] = {
+ { opal_discovery0, },
+ { start_auth_opal_session, opal_session },
+ { erase_locking_range, opal_session },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, erase_funcs);
-
- dev->func_data = data;
- dev->func_data[1] = opal_session;
- dev->func_data[2] = opal_session;
-
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2085,16 +2060,15 @@ static int opal_erase_locking_range(struct opal_dev *dev,
static int opal_enable_disable_shadow_mbr(struct opal_dev *dev,
struct opal_mbr_data *opal_mbr)
{
- void *func_data[6] = { NULL };
- static const opal_step mbr_funcs[] = {
- opal_discovery0,
- start_admin1LSP_opal_session,
- set_mbr_done,
- end_opal_session,
- start_admin1LSP_opal_session,
- set_mbr_enable_disable,
- end_opal_session,
- NULL,
+ opal_step mbr_funcs[] = {
+ { opal_discovery0, },
+ { start_admin1LSP_opal_session, &opal_mbr->key },
+ { set_mbr_done, &opal_mbr->enable_disable },
+ { end_opal_session, },
+ { start_admin1LSP_opal_session, &opal_mbr->key },
+ { set_mbr_enable_disable, &opal_mbr->enable_disable },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
@@ -2104,11 +2078,6 @@ static int opal_enable_disable_shadow_mbr(struct opal_dev *dev,
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, mbr_funcs);
- dev->func_data = func_data;
- dev->func_data[1] = &opal_mbr->key;
- dev->func_data[2] = &opal_mbr->enable_disable;
- dev->func_data[4] = &opal_mbr->key;
- dev->func_data[5] = &opal_mbr->enable_disable;
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2135,13 +2104,12 @@ static int opal_save(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk)
static int opal_add_user_to_lr(struct opal_dev *dev,
struct opal_lock_unlock *lk_unlk)
{
- void *func_data[3] = { NULL };
- static const opal_step funcs[] = {
- opal_discovery0,
- start_admin1LSP_opal_session,
- add_user_to_lr,
- end_opal_session,
- NULL
+ opal_step funcs[] = {
+ { opal_discovery0, },
+ { start_admin1LSP_opal_session, &lk_unlk->session.opal_key },
+ { add_user_to_lr, lk_unlk },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
@@ -2164,9 +2132,6 @@ static int opal_add_user_to_lr(struct opal_dev *dev,
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, funcs);
- dev->func_data = func_data;
- dev->func_data[1] = &lk_unlk->session.opal_key;
- dev->func_data[2] = lk_unlk;
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2174,55 +2139,44 @@ static int opal_add_user_to_lr(struct opal_dev *dev,
static int opal_reverttper(struct opal_dev *dev, struct opal_key *opal)
{
- void *data[2] = { NULL };
- static const opal_step revert_funcs[] = {
- opal_discovery0,
- start_SIDASP_opal_session,
- revert_tper, /* controller will terminate session */
- NULL,
+ opal_step revert_funcs[] = {
+ { opal_discovery0, },
+ { start_SIDASP_opal_session, opal },
+ { revert_tper, }, /* controller will terminate session */
+ { NULL, }
};
int ret;
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, revert_funcs);
- dev->func_data = data;
- dev->func_data[1] = opal;
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
}
-static int __opal_lock_unlock_sum(struct opal_dev *dev)
+static int __opal_lock_unlock(struct opal_dev *dev,
+ struct opal_lock_unlock *lk_unlk)
{
- static const opal_step ulk_funcs_sum[] = {
- opal_discovery0,
- start_auth_opal_session,
- lock_unlock_locking_range_sum,
- end_opal_session,
- NULL
+ opal_step _unlock_funcs[] = {
+ { opal_discovery0, },
+ { start_auth_opal_session, &lk_unlk->session },
+ { NULL, lk_unlk },
+ { end_opal_session, },
+ { NULL, }
};
- dev->funcs = ulk_funcs_sum;
- return next(dev);
-}
-
-static int __opal_lock_unlock(struct opal_dev *dev)
-{
- static const opal_step _unlock_funcs[] = {
- opal_discovery0,
- start_auth_opal_session,
- lock_unlock_locking_range,
- end_opal_session,
- NULL
- };
+ if (lk_unlk->session.sum)
+ _unlock_funcs[2].fn = lock_unlock_locking_range_sum;
+ else
+ _unlock_funcs[2].fn = lock_unlock_locking_range;
dev->funcs = _unlock_funcs;
return next(dev);
}
-static int opal_lock_unlock(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk)
+static int opal_lock_unlock(struct opal_dev *dev,
+ struct opal_lock_unlock *lk_unlk)
{
- void *func_data[3] = { NULL };
int ret;
if (lk_unlk->session.who < OPAL_ADMIN1 ||
@@ -2231,32 +2185,23 @@ static int opal_lock_unlock(struct opal_dev *dev, struct opal_lock_unlock *lk_un
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, NULL);
- dev->func_data = func_data;
- dev->func_data[1] = &lk_unlk->session;
- dev->func_data[2] = lk_unlk;
-
- if (lk_unlk->session.sum)
- ret = __opal_lock_unlock_sum(dev);
- else
- ret = __opal_lock_unlock(dev);
-
+ ret = __opal_lock_unlock(dev, lk_unlk);
mutex_unlock(&dev->dev_lock);
return ret;
}
static int opal_take_ownership(struct opal_dev *dev, struct opal_key *opal)
{
- static const opal_step owner_funcs[] = {
- opal_discovery0,
- start_anybodyASP_opal_session,
- get_msid_cpin_pin,
- end_opal_session,
- start_SIDASP_opal_session,
- set_sid_cpin_pin,
- end_opal_session,
- NULL
+ opal_step owner_funcs[] = {
+ { opal_discovery0, },
+ { start_anybodyASP_opal_session, },
+ { get_msid_cpin_pin, },
+ { end_opal_session, },
+ { start_SIDASP_opal_session, opal },
+ { set_sid_cpin_pin, opal },
+ { end_opal_session, },
+ { NULL, }
};
- void *data[6] = { NULL };
int ret;
if (!dev)
@@ -2264,9 +2209,6 @@ static int opal_take_ownership(struct opal_dev *dev, struct opal_key *opal)
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, owner_funcs);
- dev->func_data = data;
- dev->func_data[4] = opal;
- dev->func_data[5] = opal;
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2274,14 +2216,13 @@ static int opal_take_ownership(struct opal_dev *dev, struct opal_key *opal)
static int opal_activate_lsp(struct opal_dev *dev, struct opal_lr_act *opal_lr_act)
{
- void *data[4] = { NULL };
- static const opal_step active_funcs[] = {
- opal_discovery0,
- start_SIDASP_opal_session, /* Open session as SID auth */
- get_lsp_lifecycle,
- activate_lsp,
- end_opal_session,
- NULL
+ opal_step active_funcs[] = {
+ { opal_discovery0, },
+ { start_SIDASP_opal_session, &opal_lr_act->key },
+ { get_lsp_lifecycle, },
+ { activate_lsp, opal_lr_act },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
@@ -2290,9 +2231,6 @@ static int opal_activate_lsp(struct opal_dev *dev, struct opal_lr_act *opal_lr_a
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, active_funcs);
- dev->func_data = data;
- dev->func_data[1] = &opal_lr_act->key;
- dev->func_data[3] = opal_lr_act;
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2301,21 +2239,17 @@ static int opal_activate_lsp(struct opal_dev *dev, struct opal_lr_act *opal_lr_a
static int opal_setup_locking_range(struct opal_dev *dev,
struct opal_user_lr_setup *opal_lrs)
{
- void *data[3] = { NULL };
- static const opal_step lr_funcs[] = {
- opal_discovery0,
- start_auth_opal_session,
- setup_locking_range,
- end_opal_session,
- NULL,
+ opal_step lr_funcs[] = {
+ { opal_discovery0, },
+ { start_auth_opal_session, &opal_lrs->session },
+ { setup_locking_range, opal_lrs },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, lr_funcs);
- dev->func_data = data;
- dev->func_data[1] = &opal_lrs->session;
- dev->func_data[2] = opal_lrs;
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2323,14 +2257,13 @@ static int opal_setup_locking_range(struct opal_dev *dev,
static int opal_set_new_pw(struct opal_dev *dev, struct opal_new_pw *opal_pw)
{
- static const opal_step pw_funcs[] = {
- opal_discovery0,
- start_auth_opal_session,
- set_new_pw,
- end_opal_session,
- NULL
+ opal_step pw_funcs[] = {
+ { opal_discovery0, },
+ { start_auth_opal_session, &opal_pw->session },
+ { set_new_pw, &opal_pw->new_user_pw },
+ { end_opal_session, },
+ { NULL }
};
- void *data[3] = { NULL };
int ret;
if (opal_pw->session.who < OPAL_ADMIN1 ||
@@ -2341,10 +2274,6 @@ static int opal_set_new_pw(struct opal_dev *dev, struct opal_new_pw *opal_pw)
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, pw_funcs);
- dev->func_data = data;
- dev->func_data[1] = (void *) &opal_pw->session;
- dev->func_data[2] = (void *) &opal_pw->new_user_pw;
-
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2353,14 +2282,13 @@ static int opal_set_new_pw(struct opal_dev *dev, struct opal_new_pw *opal_pw)
static int opal_activate_user(struct opal_dev *dev,
struct opal_session_info *opal_session)
{
- static const opal_step act_funcs[] = {
- opal_discovery0,
- start_admin1LSP_opal_session,
- internal_activate_user,
- end_opal_session,
- NULL
+ opal_step act_funcs[] = {
+ { opal_discovery0, },
+ { start_admin1LSP_opal_session, &opal_session->opal_key },
+ { internal_activate_user, opal_session },
+ { end_opal_session, },
+ { NULL, }
};
- void *data[3] = { NULL };
int ret;
/* We can't activate Admin1 it's active as manufactured */
@@ -2372,9 +2300,6 @@ static int opal_activate_user(struct opal_dev *dev,
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, act_funcs);
- dev->func_data = data;
- dev->func_data[1] = &opal_session->opal_key;
- dev->func_data[2] = opal_session;
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2383,7 +2308,6 @@ static int opal_activate_user(struct opal_dev *dev,
bool opal_unlock_from_suspend(struct opal_dev *dev)
{
struct opal_suspend_data *suspend;
- void *func_data[3] = { NULL };
bool was_failure = false;
int ret = 0;
@@ -2394,19 +2318,13 @@ bool opal_unlock_from_suspend(struct opal_dev *dev)
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, NULL);
- dev->func_data = func_data;
list_for_each_entry(suspend, &dev->unlk_lst, node) {
dev->state = 0;
- dev->func_data[1] = &suspend->unlk.session;
- dev->func_data[2] = &suspend->unlk;
dev->tsn = 0;
dev->hsn = 0;
- if (suspend->unlk.session.sum)
- ret = __opal_lock_unlock_sum(dev);
- else
- ret = __opal_lock_unlock(dev);
+ ret = __opal_lock_unlock(dev, &suspend->unlk);
if (ret) {
pr_warn("Failed to unlock LR %hhu with sum %d\n",
suspend->unlk.session.opal_key.lr,
--
1.8.3.1
^ permalink raw reply related
* [PATCHv2 5/5] block/sed: Eliminate state variable
From: Jon Derrick @ 2017-02-18 0:00 UTC (permalink / raw)
Cc: Jon Derrick, linux-block, linux-nvme, Scott Bauer,
Rafael Antognolli, Jens Axboe, Christoph Hellwig
In-Reply-To: <1487376029-22662-1-git-send-email-jonathan.derrick@intel.com>
Now that the function data is embedded with the function, there is no
need to carry a device state variable.
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
block/sed-opal.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index e2e3228..791a40a 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -85,7 +85,6 @@ struct opal_dev {
sec_send_recv *send_recv;
opal_step *funcs;
- int state;
struct mutex dev_lock;
u16 comid;
u32 hsn;
@@ -377,17 +376,17 @@ static void check_geometry(struct opal_dev *dev, const void *data)
static int next(struct opal_dev *dev)
{
opal_step *func;
- int error = 0;
+ int state = 0, error = 0;
do {
- func = &dev->funcs[dev->state];
+ func = &dev->funcs[state];
if (!func->fn)
break;
error = func->fn(dev, func->data);
if (error) {
pr_err("Error on step function: %d with error %d: %s\n",
- dev->state, error,
+ state, error,
opal_error_to_human(error));
/* For each OPAL command we do a discovery0 then we
@@ -397,10 +396,10 @@ static int next(struct opal_dev *dev)
* session. Therefore we shouldn't attempt to terminate
* a session, as one has not yet been created.
*/
- if (dev->state > 1)
+ if (state > 1)
return end_opal_session_error(dev);
}
- dev->state++;
+ state++;
} while (!error);
return error;
@@ -1967,14 +1966,12 @@ static int end_opal_session_error(struct opal_dev *dev)
{ NULL, }
};
dev->funcs = error_end_session;
- dev->state = 0;
return next(dev);
}
static inline void setup_opal_dev(struct opal_dev *dev,
opal_step *funcs)
{
- dev->state = 0;
dev->funcs = funcs;
dev->tsn = 0;
dev->hsn = 0;
@@ -2320,7 +2317,6 @@ bool opal_unlock_from_suspend(struct opal_dev *dev)
setup_opal_dev(dev, NULL);
list_for_each_entry(suspend, &dev->unlk_lst, node) {
- dev->state = 0;
dev->tsn = 0;
dev->hsn = 0;
--
1.8.3.1
^ permalink raw reply related
* [PATCHv2 3/5] block/sed: Check received header lengths
From: Jon Derrick @ 2017-02-18 0:00 UTC (permalink / raw)
Cc: Jon Derrick, linux-block, linux-nvme, Scott Bauer,
Rafael Antognolli, Jens Axboe, Christoph Hellwig
In-Reply-To: <1487376029-22662-1-git-send-email-jonathan.derrick@intel.com>
Add a buffer size check against discovery and response header lengths
before we loop over their buffers.
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
block/sed-opal.c | 35 +++++++++++++++++++++--------------
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index d3d6db2..4fc4d7b 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -411,10 +411,17 @@ static int opal_discovery0_end(struct opal_dev *dev)
const struct d0_header *hdr = (struct d0_header *)dev->resp;
const u8 *epos = dev->resp, *cpos = dev->resp;
u16 comid = 0;
+ u32 hlen = be32_to_cpu(hdr->length);
- print_buffer(dev->resp, be32_to_cpu(hdr->length));
+ print_buffer(dev->resp, hlen);
- epos += be32_to_cpu(hdr->length); /* end of buffer */
+ if (hlen > IO_BUFFER_LENGTH - sizeof(*hdr)) {
+ pr_warn("Discovery length overflows buffer (%zu+%u)/%u\n",
+ sizeof(*hdr), hlen, IO_BUFFER_LENGTH);
+ return -EFAULT;
+ }
+
+ epos += hlen; /* end of buffer */
cpos += sizeof(*hdr); /* current position on buffer */
while (cpos < epos && supported) {
@@ -784,6 +791,7 @@ static int response_parse(const u8 *buf, size_t length,
int total;
ssize_t token_length;
const u8 *pos;
+ u32 clen, plen, slen;
if (!buf)
return -EFAULT;
@@ -795,17 +803,16 @@ static int response_parse(const u8 *buf, size_t length,
pos = buf;
pos += sizeof(*hdr);
- pr_debug("Response size: cp: %d, pkt: %d, subpkt: %d\n",
- be32_to_cpu(hdr->cp.length),
- be32_to_cpu(hdr->pkt.length),
- be32_to_cpu(hdr->subpkt.length));
-
- if (hdr->cp.length == 0 || hdr->pkt.length == 0 ||
- hdr->subpkt.length == 0) {
- pr_err("Bad header length. cp: %d, pkt: %d, subpkt: %d\n",
- be32_to_cpu(hdr->cp.length),
- be32_to_cpu(hdr->pkt.length),
- be32_to_cpu(hdr->subpkt.length));
+ clen = be32_to_cpu(hdr->cp.length);
+ plen = be32_to_cpu(hdr->pkt.length);
+ slen = be32_to_cpu(hdr->subpkt.length);
+ pr_debug("Response size: cp: %u, pkt: %u, subpkt: %u\n",
+ clen, plen, slen);
+
+ if (clen == 0 || plen == 0 || slen == 0 ||
+ slen > IO_BUFFER_LENGTH - sizeof(*hdr)) {
+ pr_err("Bad header length. cp: %u, pkt: %u, subpkt: %u\n",
+ clen, plen, slen);
print_buffer(pos, sizeof(*hdr));
return -EINVAL;
}
@@ -814,7 +821,7 @@ static int response_parse(const u8 *buf, size_t length,
return -EFAULT;
iter = resp->toks;
- total = be32_to_cpu(hdr->subpkt.length);
+ total = slen;
print_buffer(pos, total);
while (total > 0) {
if (pos[0] <= TINY_ATOM_BYTE) /* tiny atom */
--
1.8.3.1
^ permalink raw reply related
* [PATCHv2 2/5] block/sed: Add helper to qualify response tokens
From: Jon Derrick @ 2017-02-18 0:00 UTC (permalink / raw)
Cc: Jon Derrick, linux-block, linux-nvme, Scott Bauer,
Rafael Antognolli, Jens Axboe, Christoph Hellwig
In-Reply-To: <1487376029-22662-1-git-send-email-jonathan.derrick@intel.com>
Add helper which verifies the response token is valid and matches the
expected value. Merges token_type and response_get_token.
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
block/sed-opal.c | 61 +++++++++++++++++++++++---------------------------------
1 file changed, 25 insertions(+), 36 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 4675fd8..d3d6db2 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -662,48 +662,25 @@ static int cmd_finalize(struct opal_dev *cmd, u32 hsn, u32 tsn)
return 0;
}
-static enum opal_response_token token_type(const struct parsed_resp *resp,
- int n)
+static const struct opal_resp_tok *response_get_token(
+ const struct parsed_resp *resp,
+ int n)
{
const struct opal_resp_tok *tok;
if (n >= resp->num) {
pr_err("Token number doesn't exist: %d, resp: %d\n",
n, resp->num);
- return OPAL_DTA_TOKENID_INVALID;
+ return ERR_PTR(-EINVAL);
}
tok = &resp->toks[n];
if (tok->len == 0) {
pr_err("Token length must be non-zero\n");
- return OPAL_DTA_TOKENID_INVALID;
+ return ERR_PTR(-EINVAL);
}
- return tok->type;
-}
-
-/*
- * This function returns 0 in case of invalid token. One should call
- * token_type() first to find out if the token is valid or not.
- */
-static enum opal_token response_get_token(const struct parsed_resp *resp,
- int n)
-{
- const struct opal_resp_tok *tok;
-
- if (n >= resp->num) {
- pr_err("Token number doesn't exist: %d, resp: %d\n",
- n, resp->num);
- return 0;
- }
-
- tok = &resp->toks[n];
- if (tok->len == 0) {
- pr_err("Token length must be non-zero\n");
- return 0;
- }
-
- return tok->pos[0];
+ return tok;
}
static ssize_t response_parse_tiny(struct opal_resp_tok *tok,
@@ -922,20 +899,32 @@ static u64 response_get_u64(const struct parsed_resp *resp, int n)
return resp->toks[n].stored.u;
}
+static bool response_token_matches(const struct opal_resp_tok *token, u8 match)
+{
+ if (IS_ERR(token) ||
+ token->type != OPAL_DTA_TOKENID_TOKEN ||
+ token->pos[0] != match)
+ return false;
+ return true;
+}
+
static u8 response_status(const struct parsed_resp *resp)
{
- if (token_type(resp, 0) == OPAL_DTA_TOKENID_TOKEN &&
- response_get_token(resp, 0) == OPAL_ENDOFSESSION) {
+ const struct opal_resp_tok *tok;
+
+ tok = response_get_token(resp, 0);
+ if (response_token_matches(tok, OPAL_ENDOFSESSION))
return 0;
- }
if (resp->num < 5)
return DTAERROR_NO_METHOD_STATUS;
- if (token_type(resp, resp->num - 1) != OPAL_DTA_TOKENID_TOKEN ||
- token_type(resp, resp->num - 5) != OPAL_DTA_TOKENID_TOKEN ||
- response_get_token(resp, resp->num - 1) != OPAL_ENDLIST ||
- response_get_token(resp, resp->num - 5) != OPAL_STARTLIST)
+ tok = response_get_token(resp, resp->num - 5);
+ if (!response_token_matches(tok, OPAL_STARTLIST))
+ return DTAERROR_NO_METHOD_STATUS;
+
+ tok = response_get_token(resp, resp->num - 1);
+ if (!response_token_matches(tok, OPAL_ENDLIST))
return DTAERROR_NO_METHOD_STATUS;
return response_get_u64(resp, resp->num - 4);
--
1.8.3.1
^ permalink raw reply related
* [PATCHv2 1/5] block/sed: Use ssize_t on atom parsers to return errors
From: Jon Derrick @ 2017-02-18 0:00 UTC (permalink / raw)
Cc: Jon Derrick, linux-block, linux-nvme, Scott Bauer,
Rafael Antognolli, Jens Axboe, Christoph Hellwig
In-Reply-To: <1487376029-22662-1-git-send-email-jonathan.derrick@intel.com>
The short atom parser can return an errno from decoding but does not
currently return the error as a signed value. Convert all of the parsers
to ssize_t.
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
block/sed-opal.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index d1c52ba..4675fd8 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -706,8 +706,8 @@ static enum opal_token response_get_token(const struct parsed_resp *resp,
return tok->pos[0];
}
-static size_t response_parse_tiny(struct opal_resp_tok *tok,
- const u8 *pos)
+static ssize_t response_parse_tiny(struct opal_resp_tok *tok,
+ const u8 *pos)
{
tok->pos = pos;
tok->len = 1;
@@ -723,8 +723,8 @@ static size_t response_parse_tiny(struct opal_resp_tok *tok,
return tok->len;
}
-static size_t response_parse_short(struct opal_resp_tok *tok,
- const u8 *pos)
+static ssize_t response_parse_short(struct opal_resp_tok *tok,
+ const u8 *pos)
{
tok->pos = pos;
tok->len = (pos[0] & SHORT_ATOM_LEN_MASK) + 1;
@@ -736,7 +736,7 @@ static size_t response_parse_short(struct opal_resp_tok *tok,
tok->type = OPAL_DTA_TOKENID_SINT;
} else {
u64 u_integer = 0;
- int i, b = 0;
+ ssize_t i, b = 0;
tok->type = OPAL_DTA_TOKENID_UINT;
if (tok->len > 9) {
@@ -753,8 +753,8 @@ static size_t response_parse_short(struct opal_resp_tok *tok,
return tok->len;
}
-static size_t response_parse_medium(struct opal_resp_tok *tok,
- const u8 *pos)
+static ssize_t response_parse_medium(struct opal_resp_tok *tok,
+ const u8 *pos)
{
tok->pos = pos;
tok->len = (((pos[0] & MEDIUM_ATOM_LEN_MASK) << 8) | pos[1]) + 2;
@@ -770,8 +770,8 @@ static size_t response_parse_medium(struct opal_resp_tok *tok,
return tok->len;
}
-static size_t response_parse_long(struct opal_resp_tok *tok,
- const u8 *pos)
+static ssize_t response_parse_long(struct opal_resp_tok *tok,
+ const u8 *pos)
{
tok->pos = pos;
tok->len = ((pos[1] << 16) | (pos[2] << 8) | pos[3]) + 4;
@@ -787,8 +787,8 @@ static size_t response_parse_long(struct opal_resp_tok *tok,
return tok->len;
}
-static size_t response_parse_token(struct opal_resp_tok *tok,
- const u8 *pos)
+static ssize_t response_parse_token(struct opal_resp_tok *tok,
+ const u8 *pos)
{
tok->pos = pos;
tok->len = 1;
@@ -805,7 +805,7 @@ static int response_parse(const u8 *buf, size_t length,
struct opal_resp_tok *iter;
int num_entries = 0;
int total;
- size_t token_length;
+ ssize_t token_length;
const u8 *pos;
if (!buf)
@@ -851,8 +851,8 @@ static int response_parse(const u8 *buf, size_t length,
else /* TOKEN */
token_length = response_parse_token(iter, pos);
- if (token_length == -EINVAL)
- return -EINVAL;
+ if (token_length < 0)
+ return token_length;
pos += token_length;
total -= token_length;
--
1.8.3.1
^ permalink raw reply related
* [PATCHv2 0/5] OPAL patche'd cont'd
From: Jon Derrick @ 2017-02-18 0:00 UTC (permalink / raw)
Cc: Jon Derrick, linux-block, linux-nvme, Scott Bauer,
Rafael Antognolli, Jens Axboe, Christoph Hellwig
v1->v2:
Moved misplaced code from 5/5 to 4/5
The first three in the series have been reviewed already but I wanted to
squash them with the next two since they haven't been pulled yet.
The latter 2 are an attempt to refactor some of the awkward func data
and state separation into a common struct
Jon Derrick (5):
block/sed: Use ssize_t on atom parsers to return errors
block/sed: Add helper to qualify response tokens
block/sed: Check received header lengths
block/sed: Embed function data into the function sequence
block/sed: Eliminate state variable
block/sed-opal.c | 494 +++++++++++++++++++++++--------------------------------
1 file changed, 202 insertions(+), 292 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH 5/5] block/sed: Eliminate state variable
From: Jon Derrick @ 2017-02-17 22:56 UTC (permalink / raw)
Cc: Jon Derrick, linux-block, linux-nvme, Scott Bauer,
Rafael Antognolli, Jens Axboe, Christoph Hellwig
In-Reply-To: <1487372178-15715-1-git-send-email-jonathan.derrick@intel.com>
Now that the function data is embedded with the function, there is no
need to carry a device state variable.
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
block/sed-opal.c | 26 +++++++++++---------------
1 file changed, 11 insertions(+), 15 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 9fc323c..791a40a 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -85,7 +85,6 @@ struct opal_dev {
sec_send_recv *send_recv;
opal_step *funcs;
- int state;
struct mutex dev_lock;
u16 comid;
u32 hsn;
@@ -377,17 +376,17 @@ static void check_geometry(struct opal_dev *dev, const void *data)
static int next(struct opal_dev *dev)
{
opal_step *func;
- int error = 0;
+ int state = 0, error = 0;
do {
- func = &dev->funcs[dev->state];
+ func = &dev->funcs[state];
if (!func->fn)
break;
error = func->fn(dev, func->data);
if (error) {
pr_err("Error on step function: %d with error %d: %s\n",
- dev->state, error,
+ state, error,
opal_error_to_human(error));
/* For each OPAL command we do a discovery0 then we
@@ -397,10 +396,10 @@ static int next(struct opal_dev *dev)
* session. Therefore we shouldn't attempt to terminate
* a session, as one has not yet been created.
*/
- if (dev->state > 1)
+ if (state > 1)
return end_opal_session_error(dev);
}
- dev->state++;
+ state++;
} while (!error);
return error;
@@ -1285,7 +1284,7 @@ static int start_generic_opal_session(struct opal_dev *dev,
return finalize_and_send(dev, start_opal_session_cont);
}
-static int start_anybodyASP_opal_session(struct opal_dev *dev)
+static int start_anybodyASP_opal_session(struct opal_dev *dev, void *data)
{
return start_generic_opal_session(dev, OPAL_ANYBODY_UID,
OPAL_ADMINSP_UID, NULL, 0);
@@ -1376,7 +1375,7 @@ static int start_auth_opal_session(struct opal_dev *dev, void *data)
return finalize_and_send(dev, start_opal_session_cont);
}
-static int revert_tper(struct opal_dev *dev)
+static int revert_tper(struct opal_dev *dev, void *data)
{
int err = 0;
@@ -1461,7 +1460,7 @@ static int erase_locking_range(struct opal_dev *dev, void *data)
static int set_mbr_done(struct opal_dev *dev, void *data)
{
- u8 mbr_done_tf = data;
+ u8 mbr_done_tf = *(u8 *)data;
int err = 0;
clear_opal_cmd(dev);
@@ -1493,7 +1492,7 @@ static int set_mbr_done(struct opal_dev *dev, void *data)
static int set_mbr_enable_disable(struct opal_dev *dev, void *data)
{
- u8 mbr_en_dis = data;
+ u8 mbr_en_dis = *(u8 *)data;
int err = 0;
clear_opal_cmd(dev);
@@ -1962,19 +1961,17 @@ static int end_opal_session(struct opal_dev *dev, void *data)
static int end_opal_session_error(struct opal_dev *dev)
{
- const opal_step error_end_session[] = {
+ opal_step error_end_session[] = {
{ end_opal_session, },
{ NULL, }
};
dev->funcs = error_end_session;
- dev->state = 0;
return next(dev);
}
static inline void setup_opal_dev(struct opal_dev *dev,
- const opal_step *funcs)
+ opal_step *funcs)
{
- dev->state = 0;
dev->funcs = funcs;
dev->tsn = 0;
dev->hsn = 0;
@@ -2320,7 +2317,6 @@ bool opal_unlock_from_suspend(struct opal_dev *dev)
setup_opal_dev(dev, NULL);
list_for_each_entry(suspend, &dev->unlk_lst, node) {
- dev->state = 0;
dev->tsn = 0;
dev->hsn = 0;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 4/5] block/sed: embedded function data into the function sequence
From: Jon Derrick @ 2017-02-17 22:56 UTC (permalink / raw)
Cc: Jon Derrick, linux-block, linux-nvme, Scott Bauer,
Rafael Antognolli, Jens Axboe, Christoph Hellwig
In-Reply-To: <1487372178-15715-1-git-send-email-jonathan.derrick@intel.com>
By embedding the function data with the function sequence, this
eliminates a lot of the code that needs to set it in other places.
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
block/sed-opal.c | 350 +++++++++++++++++++++----------------------------------
1 file changed, 134 insertions(+), 216 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 4fc4d7b..9fc323c 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -34,7 +34,11 @@
#define IO_BUFFER_LENGTH 2048
#define MAX_TOKS 64
-typedef int (*opal_step)(struct opal_dev *dev);
+typedef struct opal_step {
+ int (*fn)(struct opal_dev *dev, void *data);
+ void *data;
+} opal_step;
+typedef int (cont_fn)(struct opal_dev *dev);
enum opal_atom_width {
OPAL_WIDTH_TINY,
@@ -80,8 +84,7 @@ struct opal_dev {
void *data;
sec_send_recv *send_recv;
- const opal_step *funcs;
- void **func_data;
+ opal_step *funcs;
int state;
struct mutex dev_lock;
u16 comid;
@@ -213,8 +216,6 @@ struct opal_dev {
{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x08, 0x03 },
};
-typedef int (cont_fn)(struct opal_dev *dev);
-
static int end_opal_session_error(struct opal_dev *dev);
struct opal_suspend_data {
@@ -375,15 +376,15 @@ static void check_geometry(struct opal_dev *dev, const void *data)
static int next(struct opal_dev *dev)
{
- opal_step func;
+ opal_step *func;
int error = 0;
do {
- func = dev->funcs[dev->state];
- if (!func)
+ func = &dev->funcs[dev->state];
+ if (!func->fn)
break;
- error = func(dev);
+ error = func->fn(dev, func->data);
if (error) {
pr_err("Error on step function: %d with error %d: %s\n",
dev->state, error,
@@ -483,7 +484,7 @@ static int opal_discovery0_end(struct opal_dev *dev)
return 0;
}
-static int opal_discovery0(struct opal_dev *dev)
+static int opal_discovery0(struct opal_dev *dev, void *data)
{
int ret;
@@ -1018,7 +1019,7 @@ static int finalize_and_send(struct opal_dev *dev, cont_fn cont)
return opal_send_recv(dev, cont);
}
-static int gen_key(struct opal_dev *dev)
+static int gen_key(struct opal_dev *dev, void *data)
{
const u8 *method;
u8 uid[OPAL_UID_LENGTH];
@@ -1072,15 +1073,14 @@ static int get_active_key_cont(struct opal_dev *dev)
return 0;
}
-static int get_active_key(struct opal_dev *dev)
+static int get_active_key(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
int err = 0;
- u8 *lr;
+ u8 *lr = data;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
- lr = dev->func_data[dev->state];
err = build_locking_range(uid, sizeof(uid), *lr);
if (err)
@@ -1163,17 +1163,16 @@ static inline int enable_global_lr(struct opal_dev *dev, u8 *uid,
return err;
}
-static int setup_locking_range(struct opal_dev *dev)
+static int setup_locking_range(struct opal_dev *dev, void *data)
{
u8 uid[OPAL_UID_LENGTH];
- struct opal_user_lr_setup *setup;
+ struct opal_user_lr_setup *setup = data;
u8 lr;
int err = 0;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
- setup = dev->func_data[dev->state];
lr = setup->session.opal_key.lr;
err = build_locking_range(uid, sizeof(uid), lr);
if (err)
@@ -1292,14 +1291,13 @@ static int start_anybodyASP_opal_session(struct opal_dev *dev)
OPAL_ADMINSP_UID, NULL, 0);
}
-static int start_SIDASP_opal_session(struct opal_dev *dev)
+static int start_SIDASP_opal_session(struct opal_dev *dev, void *data)
{
int ret;
const u8 *key = dev->prev_data;
- struct opal_key *okey;
if (!key) {
- okey = dev->func_data[dev->state];
+ const struct opal_key *okey = data;
ret = start_generic_opal_session(dev, OPAL_SID_UID,
OPAL_ADMINSP_UID,
okey->key,
@@ -1314,22 +1312,21 @@ static int start_SIDASP_opal_session(struct opal_dev *dev)
return ret;
}
-static inline int start_admin1LSP_opal_session(struct opal_dev *dev)
+static inline int start_admin1LSP_opal_session(struct opal_dev *dev, void *data)
{
- struct opal_key *key = dev->func_data[dev->state];
-
+ struct opal_key *key = data;
return start_generic_opal_session(dev, OPAL_ADMIN1_UID,
OPAL_LOCKINGSP_UID,
key->key, key->key_len);
}
-static int start_auth_opal_session(struct opal_dev *dev)
+static int start_auth_opal_session(struct opal_dev *dev, void *data)
{
+ struct opal_session_info *session = data;
u8 lk_ul_user[OPAL_UID_LENGTH];
+ size_t keylen = session->opal_key.key_len;
int err = 0;
- struct opal_session_info *session = dev->func_data[dev->state];
- size_t keylen = session->opal_key.key_len;
u8 *key = session->opal_key.key;
u32 hsn = GENERIC_HOST_SESSION_NUM;
@@ -1401,9 +1398,9 @@ static int revert_tper(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int internal_activate_user(struct opal_dev *dev)
+static int internal_activate_user(struct opal_dev *dev, void *data)
{
- struct opal_session_info *session = dev->func_data[dev->state];
+ struct opal_session_info *session = data;
u8 uid[OPAL_UID_LENGTH];
int err = 0;
@@ -1436,15 +1433,14 @@ static int internal_activate_user(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int erase_locking_range(struct opal_dev *dev)
+static int erase_locking_range(struct opal_dev *dev, void *data)
{
- struct opal_session_info *session;
+ struct opal_session_info *session = data;
u8 uid[OPAL_UID_LENGTH];
int err = 0;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
- session = dev->func_data[dev->state];
if (build_locking_range(uid, sizeof(uid), session->opal_key.lr) < 0)
return -ERANGE;
@@ -1463,9 +1459,9 @@ static int erase_locking_range(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int set_mbr_done(struct opal_dev *dev)
+static int set_mbr_done(struct opal_dev *dev, void *data)
{
- u8 mbr_done_tf = *(u8 *)dev->func_data[dev->state];
+ u8 mbr_done_tf = data;
int err = 0;
clear_opal_cmd(dev);
@@ -1495,9 +1491,9 @@ static int set_mbr_done(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int set_mbr_enable_disable(struct opal_dev *dev)
+static int set_mbr_enable_disable(struct opal_dev *dev, void *data)
{
- u8 mbr_en_dis = *(u8 *)dev->func_data[dev->state];
+ u8 mbr_en_dis = data;
int err = 0;
clear_opal_cmd(dev);
@@ -1554,11 +1550,10 @@ static int generic_pw_cmd(u8 *key, size_t key_len, u8 *cpin_uid,
return err;
}
-static int set_new_pw(struct opal_dev *dev)
+static int set_new_pw(struct opal_dev *dev, void *data)
{
u8 cpin_uid[OPAL_UID_LENGTH];
- struct opal_session_info *usr = dev->func_data[dev->state];
-
+ struct opal_session_info *usr = data;
memcpy(cpin_uid, opaluid[OPAL_C_PIN_ADMIN1], OPAL_UID_LENGTH);
@@ -1579,10 +1574,10 @@ static int set_new_pw(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int set_sid_cpin_pin(struct opal_dev *dev)
+static int set_sid_cpin_pin(struct opal_dev *dev, void *data)
{
u8 cpin_uid[OPAL_UID_LENGTH];
- struct opal_key *key = dev->func_data[dev->state];
+ struct opal_key *key = data;
memcpy(cpin_uid, opaluid[OPAL_C_PIN_SID], OPAL_UID_LENGTH);
@@ -1593,18 +1588,16 @@ static int set_sid_cpin_pin(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int add_user_to_lr(struct opal_dev *dev)
+static int add_user_to_lr(struct opal_dev *dev, void *data)
{
u8 lr_buffer[OPAL_UID_LENGTH];
u8 user_uid[OPAL_UID_LENGTH];
- struct opal_lock_unlock *lkul;
+ struct opal_lock_unlock *lkul = data;
int err = 0;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
- lkul = dev->func_data[dev->state];
-
memcpy(lr_buffer, opaluid[OPAL_LOCKINGRANGE_ACE_RDLOCKED],
OPAL_UID_LENGTH);
@@ -1671,11 +1664,11 @@ static int add_user_to_lr(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int lock_unlock_locking_range(struct opal_dev *dev)
+static int lock_unlock_locking_range(struct opal_dev *dev, void *data)
{
u8 lr_buffer[OPAL_UID_LENGTH];
const u8 *method;
- struct opal_lock_unlock *lkul;
+ struct opal_lock_unlock *lkul = data;
u8 read_locked = 1, write_locked = 1;
int err = 0;
@@ -1683,7 +1676,6 @@ static int lock_unlock_locking_range(struct opal_dev *dev)
set_comid(dev, dev->comid);
method = opalmethod[OPAL_SET];
- lkul = dev->func_data[dev->state];
if (build_locking_range(lr_buffer, sizeof(lr_buffer),
lkul->session.opal_key.lr) < 0)
return -ERANGE;
@@ -1735,19 +1727,18 @@ static int lock_unlock_locking_range(struct opal_dev *dev)
}
-static int lock_unlock_locking_range_sum(struct opal_dev *dev)
+static int lock_unlock_locking_range_sum(struct opal_dev *dev, void *data)
{
u8 lr_buffer[OPAL_UID_LENGTH];
u8 read_locked = 1, write_locked = 1;
const u8 *method;
- struct opal_lock_unlock *lkul;
+ struct opal_lock_unlock *lkul = data;
int ret;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
method = opalmethod[OPAL_SET];
- lkul = dev->func_data[dev->state];
if (build_locking_range(lr_buffer, sizeof(lr_buffer),
lkul->session.opal_key.lr) < 0)
return -ERANGE;
@@ -1778,9 +1769,9 @@ static int lock_unlock_locking_range_sum(struct opal_dev *dev)
return finalize_and_send(dev, parse_and_check_status);
}
-static int activate_lsp(struct opal_dev *dev)
+static int activate_lsp(struct opal_dev *dev, void *data)
{
- struct opal_lr_act *opal_act;
+ struct opal_lr_act *opal_act = data;
u8 user_lr[OPAL_UID_LENGTH];
u8 uint_3 = 0x83;
int err = 0, i;
@@ -1788,8 +1779,6 @@ static int activate_lsp(struct opal_dev *dev)
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
- opal_act = dev->func_data[dev->state];
-
add_token_u8(&err, dev, OPAL_CALL);
add_token_bytestring(&err, dev, opaluid[OPAL_LOCKINGSP_UID],
OPAL_UID_LENGTH);
@@ -1854,7 +1843,7 @@ static int get_lsp_lifecycle_cont(struct opal_dev *dev)
}
/* Determine if we're in the Manufactured Inactive or Active state */
-static int get_lsp_lifecycle(struct opal_dev *dev)
+static int get_lsp_lifecycle(struct opal_dev *dev, void *data)
{
int err = 0;
@@ -1915,14 +1904,13 @@ static int get_msid_cpin_pin_cont(struct opal_dev *dev)
return 0;
}
-static int get_msid_cpin_pin(struct opal_dev *dev)
+static int get_msid_cpin_pin(struct opal_dev *dev, void *data)
{
int err = 0;
clear_opal_cmd(dev);
set_comid(dev, dev->comid);
-
add_token_u8(&err, dev, OPAL_CALL);
add_token_bytestring(&err, dev, opaluid[OPAL_C_PIN_MSID],
OPAL_UID_LENGTH);
@@ -1963,7 +1951,7 @@ static int build_end_opal_session(struct opal_dev *dev)
return err;
}
-static int end_opal_session(struct opal_dev *dev)
+static int end_opal_session(struct opal_dev *dev, void *data)
{
int ret = build_end_opal_session(dev);
@@ -1975,8 +1963,8 @@ static int end_opal_session(struct opal_dev *dev)
static int end_opal_session_error(struct opal_dev *dev)
{
const opal_step error_end_session[] = {
- end_opal_session,
- NULL,
+ { end_opal_session, },
+ { NULL, }
};
dev->funcs = error_end_session;
dev->state = 0;
@@ -1990,15 +1978,14 @@ static inline void setup_opal_dev(struct opal_dev *dev,
dev->funcs = funcs;
dev->tsn = 0;
dev->hsn = 0;
- dev->func_data = NULL;
dev->prev_data = NULL;
}
static int check_opal_support(struct opal_dev *dev)
{
- static const opal_step funcs[] = {
- opal_discovery0,
- NULL
+ opal_step funcs[] = {
+ { opal_discovery0, },
+ { NULL, }
};
int ret;
@@ -2034,24 +2021,18 @@ struct opal_dev *init_opal_dev(void *data, sec_send_recv *send_recv)
static int opal_secure_erase_locking_range(struct opal_dev *dev,
struct opal_session_info *opal_session)
{
- void *data[3] = { NULL };
- static const opal_step erase_funcs[] = {
- opal_discovery0,
- start_auth_opal_session,
- get_active_key,
- gen_key,
- end_opal_session,
- NULL,
+ opal_step erase_funcs[] = {
+ { opal_discovery0, },
+ { start_auth_opal_session, opal_session },
+ { get_active_key, &opal_session->opal_key.lr },
+ { gen_key, },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, erase_funcs);
-
- dev->func_data = data;
- dev->func_data[1] = opal_session;
- dev->func_data[2] = &opal_session->opal_key.lr;
-
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2060,23 +2041,17 @@ static int opal_secure_erase_locking_range(struct opal_dev *dev,
static int opal_erase_locking_range(struct opal_dev *dev,
struct opal_session_info *opal_session)
{
- void *data[3] = { NULL };
- static const opal_step erase_funcs[] = {
- opal_discovery0,
- start_auth_opal_session,
- erase_locking_range,
- end_opal_session,
- NULL,
+ opal_step erase_funcs[] = {
+ { opal_discovery0, },
+ { start_auth_opal_session, opal_session },
+ { erase_locking_range, opal_session },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, erase_funcs);
-
- dev->func_data = data;
- dev->func_data[1] = opal_session;
- dev->func_data[2] = opal_session;
-
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2085,16 +2060,15 @@ static int opal_erase_locking_range(struct opal_dev *dev,
static int opal_enable_disable_shadow_mbr(struct opal_dev *dev,
struct opal_mbr_data *opal_mbr)
{
- void *func_data[6] = { NULL };
- static const opal_step mbr_funcs[] = {
- opal_discovery0,
- start_admin1LSP_opal_session,
- set_mbr_done,
- end_opal_session,
- start_admin1LSP_opal_session,
- set_mbr_enable_disable,
- end_opal_session,
- NULL,
+ opal_step mbr_funcs[] = {
+ { opal_discovery0, },
+ { start_admin1LSP_opal_session, &opal_mbr->key },
+ { set_mbr_done, &opal_mbr->enable_disable },
+ { end_opal_session, },
+ { start_admin1LSP_opal_session, &opal_mbr->key },
+ { set_mbr_enable_disable, &opal_mbr->enable_disable },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
@@ -2104,11 +2078,6 @@ static int opal_enable_disable_shadow_mbr(struct opal_dev *dev,
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, mbr_funcs);
- dev->func_data = func_data;
- dev->func_data[1] = &opal_mbr->key;
- dev->func_data[2] = &opal_mbr->enable_disable;
- dev->func_data[4] = &opal_mbr->key;
- dev->func_data[5] = &opal_mbr->enable_disable;
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2135,13 +2104,12 @@ static int opal_save(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk)
static int opal_add_user_to_lr(struct opal_dev *dev,
struct opal_lock_unlock *lk_unlk)
{
- void *func_data[3] = { NULL };
- static const opal_step funcs[] = {
- opal_discovery0,
- start_admin1LSP_opal_session,
- add_user_to_lr,
- end_opal_session,
- NULL
+ opal_step funcs[] = {
+ { opal_discovery0, },
+ { start_admin1LSP_opal_session, &lk_unlk->session.opal_key },
+ { add_user_to_lr, lk_unlk },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
@@ -2164,9 +2132,6 @@ static int opal_add_user_to_lr(struct opal_dev *dev,
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, funcs);
- dev->func_data = func_data;
- dev->func_data[1] = &lk_unlk->session.opal_key;
- dev->func_data[2] = lk_unlk;
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2174,55 +2139,44 @@ static int opal_add_user_to_lr(struct opal_dev *dev,
static int opal_reverttper(struct opal_dev *dev, struct opal_key *opal)
{
- void *data[2] = { NULL };
- static const opal_step revert_funcs[] = {
- opal_discovery0,
- start_SIDASP_opal_session,
- revert_tper, /* controller will terminate session */
- NULL,
+ opal_step revert_funcs[] = {
+ { opal_discovery0, },
+ { start_SIDASP_opal_session, opal },
+ { revert_tper, }, /* controller will terminate session */
+ { NULL, }
};
int ret;
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, revert_funcs);
- dev->func_data = data;
- dev->func_data[1] = opal;
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
}
-static int __opal_lock_unlock_sum(struct opal_dev *dev)
+static int __opal_lock_unlock(struct opal_dev *dev,
+ struct opal_lock_unlock *lk_unlk)
{
- static const opal_step ulk_funcs_sum[] = {
- opal_discovery0,
- start_auth_opal_session,
- lock_unlock_locking_range_sum,
- end_opal_session,
- NULL
+ opal_step _unlock_funcs[] = {
+ { opal_discovery0, },
+ { start_auth_opal_session, &lk_unlk->session },
+ { NULL, lk_unlk },
+ { end_opal_session, },
+ { NULL, }
};
- dev->funcs = ulk_funcs_sum;
- return next(dev);
-}
-
-static int __opal_lock_unlock(struct opal_dev *dev)
-{
- static const opal_step _unlock_funcs[] = {
- opal_discovery0,
- start_auth_opal_session,
- lock_unlock_locking_range,
- end_opal_session,
- NULL
- };
+ if (lk_unlk->session.sum)
+ _unlock_funcs[2].fn = lock_unlock_locking_range_sum;
+ else
+ _unlock_funcs[2].fn = lock_unlock_locking_range;
dev->funcs = _unlock_funcs;
return next(dev);
}
-static int opal_lock_unlock(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk)
+static int opal_lock_unlock(struct opal_dev *dev,
+ struct opal_lock_unlock *lk_unlk)
{
- void *func_data[3] = { NULL };
int ret;
if (lk_unlk->session.who < OPAL_ADMIN1 ||
@@ -2231,32 +2185,23 @@ static int opal_lock_unlock(struct opal_dev *dev, struct opal_lock_unlock *lk_un
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, NULL);
- dev->func_data = func_data;
- dev->func_data[1] = &lk_unlk->session;
- dev->func_data[2] = lk_unlk;
-
- if (lk_unlk->session.sum)
- ret = __opal_lock_unlock_sum(dev);
- else
- ret = __opal_lock_unlock(dev);
-
+ ret = __opal_lock_unlock(dev, lk_unlk);
mutex_unlock(&dev->dev_lock);
return ret;
}
static int opal_take_ownership(struct opal_dev *dev, struct opal_key *opal)
{
- static const opal_step owner_funcs[] = {
- opal_discovery0,
- start_anybodyASP_opal_session,
- get_msid_cpin_pin,
- end_opal_session,
- start_SIDASP_opal_session,
- set_sid_cpin_pin,
- end_opal_session,
- NULL
+ opal_step owner_funcs[] = {
+ { opal_discovery0, },
+ { start_anybodyASP_opal_session, },
+ { get_msid_cpin_pin, },
+ { end_opal_session, },
+ { start_SIDASP_opal_session, opal },
+ { set_sid_cpin_pin, opal },
+ { end_opal_session, },
+ { NULL, }
};
- void *data[6] = { NULL };
int ret;
if (!dev)
@@ -2264,9 +2209,6 @@ static int opal_take_ownership(struct opal_dev *dev, struct opal_key *opal)
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, owner_funcs);
- dev->func_data = data;
- dev->func_data[4] = opal;
- dev->func_data[5] = opal;
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2274,14 +2216,13 @@ static int opal_take_ownership(struct opal_dev *dev, struct opal_key *opal)
static int opal_activate_lsp(struct opal_dev *dev, struct opal_lr_act *opal_lr_act)
{
- void *data[4] = { NULL };
- static const opal_step active_funcs[] = {
- opal_discovery0,
- start_SIDASP_opal_session, /* Open session as SID auth */
- get_lsp_lifecycle,
- activate_lsp,
- end_opal_session,
- NULL
+ opal_step active_funcs[] = {
+ { opal_discovery0, },
+ { start_SIDASP_opal_session, &opal_lr_act->key },
+ { get_lsp_lifecycle, },
+ { activate_lsp, opal_lr_act },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
@@ -2290,9 +2231,6 @@ static int opal_activate_lsp(struct opal_dev *dev, struct opal_lr_act *opal_lr_a
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, active_funcs);
- dev->func_data = data;
- dev->func_data[1] = &opal_lr_act->key;
- dev->func_data[3] = opal_lr_act;
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2301,21 +2239,17 @@ static int opal_activate_lsp(struct opal_dev *dev, struct opal_lr_act *opal_lr_a
static int opal_setup_locking_range(struct opal_dev *dev,
struct opal_user_lr_setup *opal_lrs)
{
- void *data[3] = { NULL };
- static const opal_step lr_funcs[] = {
- opal_discovery0,
- start_auth_opal_session,
- setup_locking_range,
- end_opal_session,
- NULL,
+ opal_step lr_funcs[] = {
+ { opal_discovery0, },
+ { start_auth_opal_session, &opal_lrs->session },
+ { setup_locking_range, opal_lrs },
+ { end_opal_session, },
+ { NULL, }
};
int ret;
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, lr_funcs);
- dev->func_data = data;
- dev->func_data[1] = &opal_lrs->session;
- dev->func_data[2] = opal_lrs;
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2323,14 +2257,13 @@ static int opal_setup_locking_range(struct opal_dev *dev,
static int opal_set_new_pw(struct opal_dev *dev, struct opal_new_pw *opal_pw)
{
- static const opal_step pw_funcs[] = {
- opal_discovery0,
- start_auth_opal_session,
- set_new_pw,
- end_opal_session,
- NULL
+ opal_step pw_funcs[] = {
+ { opal_discovery0, },
+ { start_auth_opal_session, &opal_pw->session },
+ { set_new_pw, &opal_pw->new_user_pw },
+ { end_opal_session, },
+ { NULL }
};
- void *data[3] = { NULL };
int ret;
if (opal_pw->session.who < OPAL_ADMIN1 ||
@@ -2341,10 +2274,6 @@ static int opal_set_new_pw(struct opal_dev *dev, struct opal_new_pw *opal_pw)
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, pw_funcs);
- dev->func_data = data;
- dev->func_data[1] = (void *) &opal_pw->session;
- dev->func_data[2] = (void *) &opal_pw->new_user_pw;
-
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2353,14 +2282,13 @@ static int opal_set_new_pw(struct opal_dev *dev, struct opal_new_pw *opal_pw)
static int opal_activate_user(struct opal_dev *dev,
struct opal_session_info *opal_session)
{
- static const opal_step act_funcs[] = {
- opal_discovery0,
- start_admin1LSP_opal_session,
- internal_activate_user,
- end_opal_session,
- NULL
+ opal_step act_funcs[] = {
+ { opal_discovery0, },
+ { start_admin1LSP_opal_session, &opal_session->opal_key },
+ { internal_activate_user, opal_session },
+ { end_opal_session, },
+ { NULL, }
};
- void *data[3] = { NULL };
int ret;
/* We can't activate Admin1 it's active as manufactured */
@@ -2372,9 +2300,6 @@ static int opal_activate_user(struct opal_dev *dev,
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, act_funcs);
- dev->func_data = data;
- dev->func_data[1] = &opal_session->opal_key;
- dev->func_data[2] = opal_session;
ret = next(dev);
mutex_unlock(&dev->dev_lock);
return ret;
@@ -2383,7 +2308,6 @@ static int opal_activate_user(struct opal_dev *dev,
bool opal_unlock_from_suspend(struct opal_dev *dev)
{
struct opal_suspend_data *suspend;
- void *func_data[3] = { NULL };
bool was_failure = false;
int ret = 0;
@@ -2394,19 +2318,13 @@ bool opal_unlock_from_suspend(struct opal_dev *dev)
mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, NULL);
- dev->func_data = func_data;
list_for_each_entry(suspend, &dev->unlk_lst, node) {
dev->state = 0;
- dev->func_data[1] = &suspend->unlk.session;
- dev->func_data[2] = &suspend->unlk;
dev->tsn = 0;
dev->hsn = 0;
- if (suspend->unlk.session.sum)
- ret = __opal_lock_unlock_sum(dev);
- else
- ret = __opal_lock_unlock(dev);
+ ret = __opal_lock_unlock(dev, &suspend->unlk);
if (ret) {
pr_warn("Failed to unlock LR %hhu with sum %d\n",
suspend->unlk.session.opal_key.lr,
--
1.8.3.1
^ permalink raw reply related
* [PATCH 3/5] block/sed: Check received header lengths
From: Jon Derrick @ 2017-02-17 22:56 UTC (permalink / raw)
Cc: Jon Derrick, linux-block, linux-nvme, Scott Bauer,
Rafael Antognolli, Jens Axboe, Christoph Hellwig
In-Reply-To: <1487372178-15715-1-git-send-email-jonathan.derrick@intel.com>
Add a buffer size check against discovery and response header lengths
before we loop over their buffers.
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
block/sed-opal.c | 35 +++++++++++++++++++++--------------
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index d3d6db2..4fc4d7b 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -411,10 +411,17 @@ static int opal_discovery0_end(struct opal_dev *dev)
const struct d0_header *hdr = (struct d0_header *)dev->resp;
const u8 *epos = dev->resp, *cpos = dev->resp;
u16 comid = 0;
+ u32 hlen = be32_to_cpu(hdr->length);
- print_buffer(dev->resp, be32_to_cpu(hdr->length));
+ print_buffer(dev->resp, hlen);
- epos += be32_to_cpu(hdr->length); /* end of buffer */
+ if (hlen > IO_BUFFER_LENGTH - sizeof(*hdr)) {
+ pr_warn("Discovery length overflows buffer (%zu+%u)/%u\n",
+ sizeof(*hdr), hlen, IO_BUFFER_LENGTH);
+ return -EFAULT;
+ }
+
+ epos += hlen; /* end of buffer */
cpos += sizeof(*hdr); /* current position on buffer */
while (cpos < epos && supported) {
@@ -784,6 +791,7 @@ static int response_parse(const u8 *buf, size_t length,
int total;
ssize_t token_length;
const u8 *pos;
+ u32 clen, plen, slen;
if (!buf)
return -EFAULT;
@@ -795,17 +803,16 @@ static int response_parse(const u8 *buf, size_t length,
pos = buf;
pos += sizeof(*hdr);
- pr_debug("Response size: cp: %d, pkt: %d, subpkt: %d\n",
- be32_to_cpu(hdr->cp.length),
- be32_to_cpu(hdr->pkt.length),
- be32_to_cpu(hdr->subpkt.length));
-
- if (hdr->cp.length == 0 || hdr->pkt.length == 0 ||
- hdr->subpkt.length == 0) {
- pr_err("Bad header length. cp: %d, pkt: %d, subpkt: %d\n",
- be32_to_cpu(hdr->cp.length),
- be32_to_cpu(hdr->pkt.length),
- be32_to_cpu(hdr->subpkt.length));
+ clen = be32_to_cpu(hdr->cp.length);
+ plen = be32_to_cpu(hdr->pkt.length);
+ slen = be32_to_cpu(hdr->subpkt.length);
+ pr_debug("Response size: cp: %u, pkt: %u, subpkt: %u\n",
+ clen, plen, slen);
+
+ if (clen == 0 || plen == 0 || slen == 0 ||
+ slen > IO_BUFFER_LENGTH - sizeof(*hdr)) {
+ pr_err("Bad header length. cp: %u, pkt: %u, subpkt: %u\n",
+ clen, plen, slen);
print_buffer(pos, sizeof(*hdr));
return -EINVAL;
}
@@ -814,7 +821,7 @@ static int response_parse(const u8 *buf, size_t length,
return -EFAULT;
iter = resp->toks;
- total = be32_to_cpu(hdr->subpkt.length);
+ total = slen;
print_buffer(pos, total);
while (total > 0) {
if (pos[0] <= TINY_ATOM_BYTE) /* tiny atom */
--
1.8.3.1
^ permalink raw reply related
* [PATCH 2/5] block/sed: Add helper to qualify response tokens
From: Jon Derrick @ 2017-02-17 22:56 UTC (permalink / raw)
Cc: Jon Derrick, linux-block, linux-nvme, Scott Bauer,
Rafael Antognolli, Jens Axboe, Christoph Hellwig
In-Reply-To: <1487372178-15715-1-git-send-email-jonathan.derrick@intel.com>
Add helper which verifies the response token is valid and matches the
expected value. Merges token_type and response_get_token.
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
block/sed-opal.c | 61 +++++++++++++++++++++++---------------------------------
1 file changed, 25 insertions(+), 36 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 4675fd8..d3d6db2 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -662,48 +662,25 @@ static int cmd_finalize(struct opal_dev *cmd, u32 hsn, u32 tsn)
return 0;
}
-static enum opal_response_token token_type(const struct parsed_resp *resp,
- int n)
+static const struct opal_resp_tok *response_get_token(
+ const struct parsed_resp *resp,
+ int n)
{
const struct opal_resp_tok *tok;
if (n >= resp->num) {
pr_err("Token number doesn't exist: %d, resp: %d\n",
n, resp->num);
- return OPAL_DTA_TOKENID_INVALID;
+ return ERR_PTR(-EINVAL);
}
tok = &resp->toks[n];
if (tok->len == 0) {
pr_err("Token length must be non-zero\n");
- return OPAL_DTA_TOKENID_INVALID;
+ return ERR_PTR(-EINVAL);
}
- return tok->type;
-}
-
-/*
- * This function returns 0 in case of invalid token. One should call
- * token_type() first to find out if the token is valid or not.
- */
-static enum opal_token response_get_token(const struct parsed_resp *resp,
- int n)
-{
- const struct opal_resp_tok *tok;
-
- if (n >= resp->num) {
- pr_err("Token number doesn't exist: %d, resp: %d\n",
- n, resp->num);
- return 0;
- }
-
- tok = &resp->toks[n];
- if (tok->len == 0) {
- pr_err("Token length must be non-zero\n");
- return 0;
- }
-
- return tok->pos[0];
+ return tok;
}
static ssize_t response_parse_tiny(struct opal_resp_tok *tok,
@@ -922,20 +899,32 @@ static u64 response_get_u64(const struct parsed_resp *resp, int n)
return resp->toks[n].stored.u;
}
+static bool response_token_matches(const struct opal_resp_tok *token, u8 match)
+{
+ if (IS_ERR(token) ||
+ token->type != OPAL_DTA_TOKENID_TOKEN ||
+ token->pos[0] != match)
+ return false;
+ return true;
+}
+
static u8 response_status(const struct parsed_resp *resp)
{
- if (token_type(resp, 0) == OPAL_DTA_TOKENID_TOKEN &&
- response_get_token(resp, 0) == OPAL_ENDOFSESSION) {
+ const struct opal_resp_tok *tok;
+
+ tok = response_get_token(resp, 0);
+ if (response_token_matches(tok, OPAL_ENDOFSESSION))
return 0;
- }
if (resp->num < 5)
return DTAERROR_NO_METHOD_STATUS;
- if (token_type(resp, resp->num - 1) != OPAL_DTA_TOKENID_TOKEN ||
- token_type(resp, resp->num - 5) != OPAL_DTA_TOKENID_TOKEN ||
- response_get_token(resp, resp->num - 1) != OPAL_ENDLIST ||
- response_get_token(resp, resp->num - 5) != OPAL_STARTLIST)
+ tok = response_get_token(resp, resp->num - 5);
+ if (!response_token_matches(tok, OPAL_STARTLIST))
+ return DTAERROR_NO_METHOD_STATUS;
+
+ tok = response_get_token(resp, resp->num - 1);
+ if (!response_token_matches(tok, OPAL_ENDLIST))
return DTAERROR_NO_METHOD_STATUS;
return response_get_u64(resp, resp->num - 4);
--
1.8.3.1
^ permalink raw reply related
* [PATCH 1/5] block/sed: Use ssize_t on atom parsers to return errors
From: Jon Derrick @ 2017-02-17 22:56 UTC (permalink / raw)
Cc: Jon Derrick, linux-block, linux-nvme, Scott Bauer,
Rafael Antognolli, Jens Axboe, Christoph Hellwig
In-Reply-To: <1487372178-15715-1-git-send-email-jonathan.derrick@intel.com>
The short atom parser can return an errno from decoding but does not
currently return the error as a signed value. Convert all of the parsers
to ssize_t.
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
block/sed-opal.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index d1c52ba..4675fd8 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -706,8 +706,8 @@ static enum opal_token response_get_token(const struct parsed_resp *resp,
return tok->pos[0];
}
-static size_t response_parse_tiny(struct opal_resp_tok *tok,
- const u8 *pos)
+static ssize_t response_parse_tiny(struct opal_resp_tok *tok,
+ const u8 *pos)
{
tok->pos = pos;
tok->len = 1;
@@ -723,8 +723,8 @@ static size_t response_parse_tiny(struct opal_resp_tok *tok,
return tok->len;
}
-static size_t response_parse_short(struct opal_resp_tok *tok,
- const u8 *pos)
+static ssize_t response_parse_short(struct opal_resp_tok *tok,
+ const u8 *pos)
{
tok->pos = pos;
tok->len = (pos[0] & SHORT_ATOM_LEN_MASK) + 1;
@@ -736,7 +736,7 @@ static size_t response_parse_short(struct opal_resp_tok *tok,
tok->type = OPAL_DTA_TOKENID_SINT;
} else {
u64 u_integer = 0;
- int i, b = 0;
+ ssize_t i, b = 0;
tok->type = OPAL_DTA_TOKENID_UINT;
if (tok->len > 9) {
@@ -753,8 +753,8 @@ static size_t response_parse_short(struct opal_resp_tok *tok,
return tok->len;
}
-static size_t response_parse_medium(struct opal_resp_tok *tok,
- const u8 *pos)
+static ssize_t response_parse_medium(struct opal_resp_tok *tok,
+ const u8 *pos)
{
tok->pos = pos;
tok->len = (((pos[0] & MEDIUM_ATOM_LEN_MASK) << 8) | pos[1]) + 2;
@@ -770,8 +770,8 @@ static size_t response_parse_medium(struct opal_resp_tok *tok,
return tok->len;
}
-static size_t response_parse_long(struct opal_resp_tok *tok,
- const u8 *pos)
+static ssize_t response_parse_long(struct opal_resp_tok *tok,
+ const u8 *pos)
{
tok->pos = pos;
tok->len = ((pos[1] << 16) | (pos[2] << 8) | pos[3]) + 4;
@@ -787,8 +787,8 @@ static size_t response_parse_long(struct opal_resp_tok *tok,
return tok->len;
}
-static size_t response_parse_token(struct opal_resp_tok *tok,
- const u8 *pos)
+static ssize_t response_parse_token(struct opal_resp_tok *tok,
+ const u8 *pos)
{
tok->pos = pos;
tok->len = 1;
@@ -805,7 +805,7 @@ static int response_parse(const u8 *buf, size_t length,
struct opal_resp_tok *iter;
int num_entries = 0;
int total;
- size_t token_length;
+ ssize_t token_length;
const u8 *pos;
if (!buf)
@@ -851,8 +851,8 @@ static int response_parse(const u8 *buf, size_t length,
else /* TOKEN */
token_length = response_parse_token(iter, pos);
- if (token_length == -EINVAL)
- return -EINVAL;
+ if (token_length < 0)
+ return token_length;
pos += token_length;
total -= token_length;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 0/5] OPAL patches cont'd
From: Jon Derrick @ 2017-02-17 22:56 UTC (permalink / raw)
Cc: Jon Derrick, linux-block, linux-nvme, Scott Bauer,
Rafael Antognolli, Jens Axboe, Christoph Hellwig
The first three in the series have been reviewed already but I wanted to
squash them with the next two since they haven't been pulled yet.
The latter 2 are an attempt to refactor some of the awkward func data
and state separation into a common struct
Jon Derrick (5):
block/sed: Use ssize_t on atom parsers to return errors
block/sed: Add helper to qualify response tokens
block/sed: Check received header lengths
block/sed: embedded function data into the function sequence
block/sed: Eliminate state variable
block/sed-opal.c | 494 +++++++++++++++++++++++--------------------------------
1 file changed, 202 insertions(+), 292 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [GIT PULL] Single block fix for 4.10
From: Jens Axboe @ 2017-02-17 19:50 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel@vger.kernel.org, linux-block@vger.kernel.org
Hi Linus,
Single fix in this pull request, fixing a lockdep splat reported by
Thomas and Gabriel. Please pull for 4.10 final! Thanks.
git://git.kernel.dk/linux-block.git for-linus
----------------------------------------------------------------
Jens Axboe (1):
cfq-iosched: don't call wbt_disable_default() with IRQs disabled
block/cfq-iosched.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
--
Jens Axboe
^ permalink raw reply
* Re: OPAL fixups
From: Jens Axboe @ 2017-02-17 19:42 UTC (permalink / raw)
To: Christoph Hellwig, scott.bauer, keith.busch, jonathan.derrick
Cc: linux-block, linux-nvme
In-Reply-To: <20170217125941.14319-1-hch@lst.de>
On 02/17/2017 05:59 AM, Christoph Hellwig wrote:
> Hi all,
>
> this contains a few more OPAL-related fixups. It tones down warnings a bit,
> allocates the OPAL-ѕpecific data structure in a separate dynamic allocation,
> checks for support of Security Send/Receive in NVMe before using them,
> and makes sure we re-discovery the security capabilities after each reset.
Applied 1-3, thanks.
--
Jens Axboe
^ permalink raw reply
* [PATCH] block/sed-opal: Introduce free_opal_dev to free the structure and clean up state
From: Scott Bauer @ 2017-02-17 17:25 UTC (permalink / raw)
To: linux-block; +Cc: axboe, keith.busch, jonathan.derrick, hch, Scott Bauer
Before we free the opal structure we need to clean up any saved
locking ranges that the user had told us to unlock from a suspend.
Also fixup a list_for_each to list_for_each_safe in the save path.
Signed-off-by: Scott Bauer <scott.bauer@intel.com>
---
block/sed-opal.c | 33 +++++++++++++++++++++++++++++++--
include/linux/sed-opal.h | 5 +++++
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index d1c52ba..1be5c72 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -988,9 +988,9 @@ static int start_opal_session_cont(struct opal_dev *dev)
static void add_suspend_info(struct opal_dev *dev,
struct opal_suspend_data *sus)
{
- struct opal_suspend_data *iter;
+ struct opal_suspend_data *iter, *next;
- list_for_each_entry(iter, &dev->unlk_lst, node) {
+ list_for_each_entry_safe(iter, next, &dev->unlk_lst, node) {
if (iter->lr == sus->lr) {
list_del(&iter->node);
kfree(iter);
@@ -2014,6 +2014,28 @@ static int check_opal_support(struct opal_dev *dev)
return ret;
}
+static void clean_opal_dev(struct opal_dev *dev)
+{
+
+ struct opal_suspend_data *suspend, *next;
+
+ mutex_lock(&dev->dev_lock);
+ list_for_each_entry_safe(suspend, next, &dev->unlk_lst, node) {
+ list_del(&suspend->node);
+ kfree(suspend);
+ }
+ mutex_unlock(&dev->dev_lock);
+}
+
+void free_opal_dev(struct opal_dev *dev)
+{
+ if (!dev)
+ return;
+ clean_opal_dev(dev);
+ kfree(dev);
+}
+EXPORT_SYMBOL(free_opal_dev);
+
struct opal_dev *init_opal_dev(void *data, sec_send_recv *send_recv)
{
struct opal_dev *dev;
@@ -2193,6 +2215,13 @@ static int opal_reverttper(struct opal_dev *dev, struct opal_key *opal)
dev->func_data[1] = opal;
ret = next(dev);
mutex_unlock(&dev->dev_lock);
+
+ /* If we succesfully reverted lets clean
+ * any saved locking ranges.
+ */
+ if (!ret)
+ clean_opal_dev(dev);
+
return ret;
}
diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index deee23d..04b124f 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -27,6 +27,7 @@ typedef int (sec_send_recv)(void *data, u16 spsp, u8 secp, void *buffer,
size_t len, bool send);
#ifdef CONFIG_BLK_SED_OPAL
+void free_opal_dev(struct opal_dev *dev);
bool opal_unlock_from_suspend(struct opal_dev *dev);
struct opal_dev *init_opal_dev(void *data, sec_send_recv *send_recv);
int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *ioctl_ptr);
@@ -51,6 +52,10 @@ static inline bool is_sed_ioctl(unsigned int cmd)
return false;
}
#else
+static inline void free_opal_dev(struct opal_dev *dev)
+{
+}
+
static inline bool is_sed_ioctl(unsigned int cmd)
{
return false;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 4/4] nvme: re-check security protocol support after reset
From: Christoph Hellwig @ 2017-02-17 17:16 UTC (permalink / raw)
To: Scott Bauer
Cc: Christoph Hellwig, Keith Busch, jonathan.derrick, axboe,
linux-block, linux-nvme
In-Reply-To: <20170217165551.GA2062@sbauer-Z170X-UD5>
On Fri, Feb 17, 2017 at 09:55:51AM -0700, Scott Bauer wrote:
> I'm working on it now. Do you want a diff like Keith did or a separate patch?
Please make it a proper patch that applies on top of my patches 1-3 (where
3 really is yours anyway). I'll respin 4 with the updates from Keith on
top of 1-3 + your patch then.
^ permalink raw reply
* Re: [PATCH 4/4] nvme: re-check security protocol support after reset
From: Scott Bauer @ 2017-02-17 16:55 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Keith Busch, jonathan.derrick, axboe, linux-block, linux-nvme
In-Reply-To: <20170217170128.GA29208@lst.de>
On Fri, Feb 17, 2017 at 06:01:28PM +0100, Christoph Hellwig wrote:
> On Fri, Feb 17, 2017 at 10:26:51AM -0500, Keith Busch wrote:
> > On Fri, Feb 17, 2017 at 01:59:41PM +0100, Christoph Hellwig wrote:
> > > @@ -1789,7 +1789,8 @@ static void nvme_reset_work(struct work_struct *work)
> > > if (result)
> > > goto out;
> > >
> > > - if ((dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) && !dev->ctrl.opal_dev) {
> > > + kfree(dev->ctrl.opal_dev);
> > > + if (dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) {
> > > dev->ctrl.opal_dev =
> > > init_opal_dev(&dev->ctrl, &nvme_sec_submit);
> > > }
> >
> > A couple things.
> >
> > This has a use-after-free in opal_unlock_from_suspend if the nvme
> > device had an opal_dev before, but no longer support the capability
> > after resume. So you'd want to set ctrl.opal_dev to NULL after the free.
> >
> > But we don't want to unconditionally free it anyway during resume
> > since opal_unlock_from_suspend requires the exisiting opal_dev state
> > information saved in the 'unlk_list'.
> >
> > Something like this instead:
>
> Yes, that looks fine to me. We'll probably also need the additional
> fixup Scott pointed out.
I'm working on it now. Do you want a diff like Keith did or a separate patch?
^ permalink raw reply
* Re: [PATCH 4/4] nvme: re-check security protocol support after reset
From: Christoph Hellwig @ 2017-02-17 17:01 UTC (permalink / raw)
To: Keith Busch
Cc: Christoph Hellwig, scott.bauer, jonathan.derrick, axboe,
linux-block, linux-nvme
In-Reply-To: <20170217152651.GA18275@localhost.localdomain>
On Fri, Feb 17, 2017 at 10:26:51AM -0500, Keith Busch wrote:
> On Fri, Feb 17, 2017 at 01:59:41PM +0100, Christoph Hellwig wrote:
> > @@ -1789,7 +1789,8 @@ static void nvme_reset_work(struct work_struct *work)
> > if (result)
> > goto out;
> >
> > - if ((dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) && !dev->ctrl.opal_dev) {
> > + kfree(dev->ctrl.opal_dev);
> > + if (dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) {
> > dev->ctrl.opal_dev =
> > init_opal_dev(&dev->ctrl, &nvme_sec_submit);
> > }
>
> A couple things.
>
> This has a use-after-free in opal_unlock_from_suspend if the nvme
> device had an opal_dev before, but no longer support the capability
> after resume. So you'd want to set ctrl.opal_dev to NULL after the free.
>
> But we don't want to unconditionally free it anyway during resume
> since opal_unlock_from_suspend requires the exisiting opal_dev state
> information saved in the 'unlk_list'.
>
> Something like this instead:
Yes, that looks fine to me. We'll probably also need the additional
fixup Scott pointed out.
^ permalink raw reply
* RE: [PATCHv3 4/4] MAINTAINERS: Remove powerpc's opal match
From: Elliott, Robert (Persistent Memory) @ 2017-02-17 16:40 UTC (permalink / raw)
To: Jon Derrick, Michael Ellerman
Cc: Jens Axboe, Rafael Antognolli, Greg Kroah-Hartman,
linux-kernel@vger.kernel.org, linux-block@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, Christoph Hellwig, Scott Bauer
In-Reply-To: <64674df8-8e5e-07a0-8062-d02e588b2a32@intel.com>
> -----Original Message-----
> From: linux-block-owner@vger.kernel.org [mailto:linux-block-
> owner@vger.kernel.org] On Behalf Of Jon Derrick
> Sent: Thursday, February 16, 2017 10:15 AM
> To: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Jens Axboe <axboe@kernel.dk>; Rafael Antognolli
> <rafael.antognolli@intel.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org; linux-
> block@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Christoph Hellwig
> <hch@lst.de>; Scott Bauer <scott.bauer@intel.com>
> Subject: Re: [PATCHv3 4/4] MAINTAINERS: Remove powerpc's opal match
>=20
> Thanks everyone. Sorry about the mess :)
>=20
> On 02/15/2017 10:23 PM, Michael Ellerman wrote:
> > Jon Derrick <jonathan.derrick@intel.com> writes:
> >
> >> PPC's 'opal' match pattern also matches block/sed-opal.c, where it loo=
ks
> >> like the 'arch/powerpc' file pattern should be enough to match powerpc
> >> opal code by itself. Remove the opal regex pattern from powerpc.
> >
> > We thought of it first.
> >
> > Can't you just rename your driver, Opal Storage Specification, so "oss"=
,
> > that should be pretty unique?
> >
> > ... :)
The library could easily be used for devices supporting the Opalite and Pyr=
ite=20
SSCs, not just the Opal SSC. With some effort, I suspect that Enterprise SS=
C
could also be supported. So, a broader name might indeed be useful.
The full names of the specifications are:
TCG Storage Security Subsystem Class: Opal
TCG Storage Security Subsystem Class: Opalite
TCG Storage Security Subsystem Class: Pyrite
TCG Storage Security Subsystem Class: Enterprise
---
Robert Elliott, HPE Persistent Memory
^ permalink raw reply
* Re: [PATCH 4/4] nvme: re-check security protocol support after reset
From: Scott Bauer @ 2017-02-17 15:41 UTC (permalink / raw)
To: Keith Busch
Cc: Christoph Hellwig, jonathan.derrick, axboe, linux-block,
linux-nvme
In-Reply-To: <20170217152651.GA18275@localhost.localdomain>
On Fri, Feb 17, 2017 at 10:26:51AM -0500, Keith Busch wrote:
> On Fri, Feb 17, 2017 at 01:59:41PM +0100, Christoph Hellwig wrote:
> > @@ -1789,7 +1789,8 @@ static void nvme_reset_work(struct work_struct *work)
> > if (result)
> > goto out;
> >
> > - if ((dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) && !dev->ctrl.opal_dev) {
> > + kfree(dev->ctrl.opal_dev);
> > + if (dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) {
> > dev->ctrl.opal_dev =
> > init_opal_dev(&dev->ctrl, &nvme_sec_submit);
> > }
>
> A couple things.
>
> This has a use-after-free in opal_unlock_from_suspend if the nvme
> device had an opal_dev before, but no longer support the capability
> after resume. So you'd want to set ctrl.opal_dev to NULL after the free.
>
> But we don't want to unconditionally free it anyway during resume
> since opal_unlock_from_suspend requires the exisiting opal_dev state
> information saved in the 'unlk_list'.
>
> Something like this instead:
>
> ---
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index ddc51ad..8fa6be9 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -1789,13 +1789,17 @@ static void nvme_reset_work(struct work_struct *work)
> if (result)
> goto out;
>
> - if ((dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) && !dev->ctrl.opal_dev) {
> - dev->ctrl.opal_dev =
> - init_opal_dev(&dev->ctrl, &nvme_sec_submit);
> + if (dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP)
> + if (was_suspend && dev->ctrl.opal_dev)
> + opal_unlock_from_suspend(dev->ctrl.opal_dev);
> + else if (!dev->ctrl.opal_dev)
> + dev->ctrl.opal_dev =
> + init_opal_dev(&dev->ctrl, &nvme_sec_submit);
> + } else {
> + kfree(dev->ctrl.opal_dev);
> + dev->ctrl.opal_dev = NULL;
Keith's comments made me realize something even deeper as well. Assuming the firmware
changed and we no longer support security commands we need to free the opal_dev structure
like we're doing but there is a possiblity that there were saved ranges in the structure
that we need to free as well. If the user had previously told the kernel to unlock 5
ranges coming out of a suspend there are 5 structures we need to free inside the opal_dev
before we free the opal dev. We'll need to re-introduce free_opal_dev() in the opal code
like we had a while back.
^ permalink raw reply
* Re: OPAL fixups
From: Keith Busch @ 2017-02-17 15:27 UTC (permalink / raw)
To: Christoph Hellwig
Cc: scott.bauer, jonathan.derrick, axboe, linux-block, linux-nvme
In-Reply-To: <20170217125941.14319-1-hch@lst.de>
On Fri, Feb 17, 2017 at 01:59:37PM +0100, Christoph Hellwig wrote:
> Hi all,
>
> this contains a few more OPAL-related fixups. It tones down warnings a bit,
> allocates the OPAL-ѕpecific data structure in a separate dynamic allocation,
> checks for support of Security Send/Receive in NVMe before using them,
> and makes sure we re-discovery the security capabilities after each reset.
Patches 1-3 look good to me.
^ permalink raw reply
* Re: [PATCH 4/4] nvme: re-check security protocol support after reset
From: Keith Busch @ 2017-02-17 15:26 UTC (permalink / raw)
To: Christoph Hellwig
Cc: scott.bauer, jonathan.derrick, axboe, linux-block, linux-nvme
In-Reply-To: <20170217125941.14319-5-hch@lst.de>
On Fri, Feb 17, 2017 at 01:59:41PM +0100, Christoph Hellwig wrote:
> @@ -1789,7 +1789,8 @@ static void nvme_reset_work(struct work_struct *work)
> if (result)
> goto out;
>
> - if ((dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) && !dev->ctrl.opal_dev) {
> + kfree(dev->ctrl.opal_dev);
> + if (dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) {
> dev->ctrl.opal_dev =
> init_opal_dev(&dev->ctrl, &nvme_sec_submit);
> }
A couple things.
This has a use-after-free in opal_unlock_from_suspend if the nvme
device had an opal_dev before, but no longer support the capability
after resume. So you'd want to set ctrl.opal_dev to NULL after the free.
But we don't want to unconditionally free it anyway during resume
since opal_unlock_from_suspend requires the exisiting opal_dev state
information saved in the 'unlk_list'.
Something like this instead:
---
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index ddc51ad..8fa6be9 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1789,13 +1789,17 @@ static void nvme_reset_work(struct work_struct *work)
if (result)
goto out;
- if ((dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) && !dev->ctrl.opal_dev) {
- dev->ctrl.opal_dev =
- init_opal_dev(&dev->ctrl, &nvme_sec_submit);
+ if (dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP)
+ if (was_suspend && dev->ctrl.opal_dev)
+ opal_unlock_from_suspend(dev->ctrl.opal_dev);
+ else if (!dev->ctrl.opal_dev)
+ dev->ctrl.opal_dev =
+ init_opal_dev(&dev->ctrl, &nvme_sec_submit);
+ } else {
+ kfree(dev->ctrl.opal_dev);
+ dev->ctrl.opal_dev = NULL;
}
- if (was_suspend)
- opal_unlock_from_suspend(dev->ctrl.opal_dev);
result = nvme_setup_io_queues(dev);
if (result)
--
^ permalink raw reply related
* Re: Some throughput tests with MQ and BFQ on MMC/SD
From: Linus Walleij @ 2017-02-17 13:22 UTC (permalink / raw)
To: Ziji Hu
Cc: linux-mmc@vger.kernel.org, linux-block, Ulf Hansson,
Adrian Hunter, Ritesh Harjani, Avri Altman, Arnd Bergmann,
Christoph Hellwig, Jens Axboe, Paolo Valente, Per Förlin
In-Reply-To: <e4836ece-d990-667c-27d0-4b19a2b4b160@marvell.com>
On Fri, Feb 17, 2017 at 12:53 PM, Ziji Hu <huziji@marvell.com> wrote:
> I would like to suggest that you should try the multiple thread
> test mode of iozone, since you are testing *Multi* Queue.
Good point. This target has only 2 CPUs but still, maybe it performs!
> Besides, it seems that your eMMC transfer speed is quite low.
> It is normal that read speed can reach more than 100MB/s in HS400.
> Could you try a higher speed mode? The test result might be
> limited by the bus clock frequency.
The iozone tests are done on an SDcard. And I only did read tests on
the eMMC I have.
It's because I'm afriad of wearing out my eMMC :(
But OK I'll just take the risk and run iozone on the eMMC.
> Actually I have been following your thread for some time.
> But currently I'm a little confused.
> May I know the purpose of your patch?
Ulf describes it: we want to switch MMC/SD to MQ.
To me, there are two reasons for that (no secret agendas...)
1. To get away from the legacy codebase in the old block layer.
Christoph and Jens have been very clear stating that the old block
layer is in maintenance mode and should be phased out, and they
asked explicitly for out help to do so. Currently
MMC/SD is a big fat roadblock to these plans so it is win-win for
MMC/SD and the block layer if we can just switch over to MQ.
2. My colleague Paolo Valente is working on the next generation
block scheduler BFQ which has very promising potential for
interactive loads. (Like taking a backup of your harddrive while
playing 1080p video let's say.) Since the
old block layer is no longer maintained, this scheduler will only
be merged and made available for systems deploying MQ. He's
already working full steam on that.
I would like to make 1+2 happen in the next merge window
ultimately, but yeah, maybe I'm overly optimistic. But I will sure
try.
Maybe I should add:
3. MQ is a better and future-proof fit for command queueing.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH 4/4] nvme: re-check security protocol support after reset
From: Christoph Hellwig @ 2017-02-17 12:59 UTC (permalink / raw)
To: scott.bauer, keith.busch, jonathan.derrick, axboe; +Cc: linux-block, linux-nvme
In-Reply-To: <20170217125941.14319-1-hch@lst.de>
A device may change capabilities after each reset, e.g. due to a firmware
upgrade. We should thus check for Security Send/Receive and OPAL support
after each reset.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/pci.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index ddc51adb594d..c5986850f88b 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1789,7 +1789,8 @@ static void nvme_reset_work(struct work_struct *work)
if (result)
goto out;
- if ((dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) && !dev->ctrl.opal_dev) {
+ kfree(dev->ctrl.opal_dev);
+ if (dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) {
dev->ctrl.opal_dev =
init_opal_dev(&dev->ctrl, &nvme_sec_submit);
}
--
2.11.0
^ permalink raw reply related
* [PATCH 3/4] nvme: Check for Security send/recv support before issuing commands.
From: Christoph Hellwig @ 2017-02-17 12:59 UTC (permalink / raw)
To: scott.bauer, keith.busch, jonathan.derrick, axboe; +Cc: linux-block, linux-nvme
In-Reply-To: <20170217125941.14319-1-hch@lst.de>
From: Scott Bauer <scott.bauer@intel.com>
We need to verify that the controller supports the security
commands before actually trying to issue them.
Signed-off-by: Scott Bauer <scott.bauer@intel.com>
[hch: moved the check so that we don't call into the OPAL code if not
supported]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/core.c | 1 +
drivers/nvme/host/nvme.h | 1 +
drivers/nvme/host/pci.c | 2 +-
include/linux/nvme.h | 1 +
4 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 8aeb4a623b65..44a1a257e0b5 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1285,6 +1285,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
return -EIO;
}
+ ctrl->oacs = le16_to_cpu(id->oacs);
ctrl->vid = le16_to_cpu(id->vid);
ctrl->oncs = le16_to_cpup(&id->oncs);
atomic_set(&ctrl->abort_limit, id->acl + 1);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 5126c4bbee1a..14cfc6f7facb 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -140,6 +140,7 @@ struct nvme_ctrl {
u32 max_hw_sectors;
u16 oncs;
u16 vid;
+ u16 oacs;
atomic_t abort_limit;
u8 event_limit;
u8 vwc;
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 5db8a38a8b43..ddc51adb594d 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1789,7 +1789,7 @@ static void nvme_reset_work(struct work_struct *work)
if (result)
goto out;
- if (!dev->ctrl.opal_dev) {
+ if ((dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) && !dev->ctrl.opal_dev) {
dev->ctrl.opal_dev =
init_opal_dev(&dev->ctrl, &nvme_sec_submit);
}
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index 3e2ed49c3ad8..0b676a02cf3e 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -244,6 +244,7 @@ enum {
NVME_CTRL_ONCS_DSM = 1 << 2,
NVME_CTRL_ONCS_WRITE_ZEROES = 1 << 3,
NVME_CTRL_VWC_PRESENT = 1 << 0,
+ NVME_CTRL_OACS_SEC_SUPP = 1 << 0,
};
struct nvme_lbaf {
--
2.11.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox