Skip to content

Commit

Permalink
block: sed-opal: pass steps via argument rather than via opal_dev
Browse files Browse the repository at this point in the history 
The steps argument is only read by the next function, so it can
be passed directly as an argument rather than via opal_dev.

Normally, the steps is an array on the stack, so the pointer stops
being valid then the function that set opal_dev.steps returns.
If opal_dev.steps was not set to NULL before return it would become
a dangling pointer. When the steps are passed as argument this
becomes easier to see and more difficult to misuse.

Acked-by: Jon Derrick <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Reviewed-by: Scott Bauer <[email protected]>
Signed-off-by: David Kozub <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
  • Loading branch information
@axboe
David Kozub authored and axboe committed Apr 6, 2019
1 parent 372be40 commit 3db8723
Showing 1 changed file with 69 additions and 89 deletions.
158 changes: 69 additions & 89 deletions block/sed-opal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ struct opal_dev {
void *data;
sec_send_recv *send_recv;

const struct opal_step *steps;
struct mutex dev_lock;
u16 comid;
u32 hsn;
Expand Down Expand Up @@ -382,37 +381,34 @@ static void check_geometry(struct opal_dev *dev, const void *data)
dev->lowest_lba = geo->lowest_aligned_lba;
}

static int next(struct opal_dev *dev)
static int next(struct opal_dev *dev, const struct opal_step *steps,
size_t n_steps)
{
const struct opal_step *step;
int state = 0, error = 0;
size_t state;
int error = 0;

do {
step = &dev->steps[state];
if (!step->fn)
break;
for (state = 0; state < n_steps; state++) {
step = &steps[state];

error = step->fn(dev, step->data);
if (error) {
pr_debug("Step %d (%pS) failed with error %d: %s\n",
state, step->fn, error,
opal_error_to_human(error));

/* For each OPAL command we do a discovery0 then we
* start some sort of session.
* If we haven't passed state 1 then there was an error
* on discovery0 or during the attempt to start a
* session. Therefore we shouldn't attempt to terminate
* a session, as one has not yet been created.
*/
if (state > 1) {
end_opal_session_error(dev);
return error;
}
if (error)
goto out_error;
}

}
state++;
} while (!error);
return 0;

out_error:
/*
* For each OPAL command the first step in steps does a discovery0
* and the second step starts some sort of session. If an error occurred
* in the first two steps (and thus stopping the loop with state <= 1)
* then there was an error before or during the attempt to start a
* session. Therefore we shouldn't attempt to terminate a session, as
* one has not yet been created.
*/
if (state > 1)
end_opal_session_error(dev);

return error;
}
Expand Down Expand Up @@ -1836,17 +1832,13 @@ static int end_opal_session(struct opal_dev *dev, void *data)
static int end_opal_session_error(struct opal_dev *dev)
{
const struct opal_step error_end_session[] = {
{ end_opal_session, },
{ NULL, }
{ end_opal_session, }
};
dev->steps = error_end_session;
return next(dev);
return next(dev, error_end_session, ARRAY_SIZE(error_end_session));
}

static inline void setup_opal_dev(struct opal_dev *dev,
const struct opal_step *steps)
static inline void setup_opal_dev(struct opal_dev *dev)
{
dev->steps = steps;
dev->tsn = 0;
dev->hsn = 0;
dev->prev_data = NULL;
Expand All @@ -1855,14 +1847,13 @@ static inline void setup_opal_dev(struct opal_dev *dev,
static int check_opal_support(struct opal_dev *dev)
{
const struct opal_step steps[] = {
{ opal_discovery0, },
{ NULL, }
{ opal_discovery0, }
};
int ret;

mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, steps);
ret = next(dev);
setup_opal_dev(dev);
ret = next(dev, steps, ARRAY_SIZE(steps));
dev->supported = !ret;
mutex_unlock(&dev->dev_lock);
return ret;
Expand Down Expand Up @@ -1919,14 +1910,13 @@ static int opal_secure_erase_locking_range(struct opal_dev *dev,
{ start_auth_opal_session, opal_session },
{ get_active_key, &opal_session->opal_key.lr },
{ gen_key, },
{ end_opal_session, },
{ NULL, }
{ end_opal_session, }
};
int ret;

mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, erase_steps);
ret = next(dev);
setup_opal_dev(dev);
ret = next(dev, erase_steps, ARRAY_SIZE(erase_steps));
mutex_unlock(&dev->dev_lock);
return ret;
}
Expand All @@ -1938,14 +1928,13 @@ static int opal_erase_locking_range(struct opal_dev *dev,
{ opal_discovery0, },
{ start_auth_opal_session, opal_session },
{ erase_locking_range, opal_session },
{ end_opal_session, },
{ NULL, }
{ end_opal_session, }
};
int ret;

mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, erase_steps);
ret = next(dev);
setup_opal_dev(dev);
ret = next(dev, erase_steps, ARRAY_SIZE(erase_steps));
mutex_unlock(&dev->dev_lock);
return ret;
}
Expand All @@ -1963,8 +1952,7 @@ static int opal_enable_disable_shadow_mbr(struct opal_dev *dev,
{ end_opal_session, },
{ start_admin1LSP_opal_session, &opal_mbr->key },
{ set_mbr_enable_disable, &enable_disable },
{ end_opal_session, },
{ NULL, }
{ end_opal_session, }
};
int ret;

Expand All @@ -1973,8 +1961,8 @@ static int opal_enable_disable_shadow_mbr(struct opal_dev *dev,
return -EINVAL;

mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, mbr_steps);
ret = next(dev);
setup_opal_dev(dev);
ret = next(dev, mbr_steps, ARRAY_SIZE(mbr_steps));
mutex_unlock(&dev->dev_lock);
return ret;
}
Expand All @@ -1991,7 +1979,7 @@ static int opal_save(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk)
suspend->lr = lk_unlk->session.opal_key.lr;

mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, NULL);
setup_opal_dev(dev);
add_suspend_info(dev, suspend);
mutex_unlock(&dev->dev_lock);
return 0;
Expand All @@ -2004,8 +1992,7 @@ static int opal_add_user_to_lr(struct opal_dev *dev,
{ opal_discovery0, },
{ start_admin1LSP_opal_session, &lk_unlk->session.opal_key },
{ add_user_to_lr, lk_unlk },
{ end_opal_session, },
{ NULL, }
{ end_opal_session, }
};
int ret;

Expand All @@ -2027,8 +2014,8 @@ static int opal_add_user_to_lr(struct opal_dev *dev,
}

mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, steps);
ret = next(dev);
setup_opal_dev(dev);
ret = next(dev, steps, ARRAY_SIZE(steps));
mutex_unlock(&dev->dev_lock);
return ret;
}
Expand All @@ -2038,14 +2025,13 @@ static int opal_reverttper(struct opal_dev *dev, struct opal_key *opal)
const struct opal_step revert_steps[] = {
{ opal_discovery0, },
{ start_SIDASP_opal_session, opal },
{ revert_tper, }, /* controller will terminate session */
{ NULL, }
{ revert_tper, } /* controller will terminate session */
};
int ret;

mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, revert_steps);
ret = next(dev);
setup_opal_dev(dev);
ret = next(dev, revert_steps, ARRAY_SIZE(revert_steps));
mutex_unlock(&dev->dev_lock);

/*
Expand All @@ -2065,19 +2051,20 @@ static int __opal_lock_unlock(struct opal_dev *dev,
{ opal_discovery0, },
{ start_auth_opal_session, &lk_unlk->session },
{ lock_unlock_locking_range, lk_unlk },
{ end_opal_session, },
{ NULL, }
{ end_opal_session, }
};
const struct opal_step unlock_sum_steps[] = {
{ opal_discovery0, },
{ start_auth_opal_session, &lk_unlk->session },
{ lock_unlock_locking_range_sum, lk_unlk },
{ end_opal_session, },
{ NULL, }
{ end_opal_session, }
};

dev->steps = lk_unlk->session.sum ? unlock_sum_steps : unlock_steps;
return next(dev);
if (lk_unlk->session.sum)
return next(dev, unlock_sum_steps,
ARRAY_SIZE(unlock_sum_steps));
else
return next(dev, unlock_steps, ARRAY_SIZE(unlock_steps));
}

static int __opal_set_mbr_done(struct opal_dev *dev, struct opal_key *key)
Expand All @@ -2087,12 +2074,10 @@ static int __opal_set_mbr_done(struct opal_dev *dev, struct opal_key *key)
{ opal_discovery0, },
{ start_admin1LSP_opal_session, key },
{ set_mbr_done, &mbr_done_tf },
{ end_opal_session, },
{ NULL, }
{ end_opal_session, }
};

dev->steps = mbrdone_step;
return next(dev);
return next(dev, mbrdone_step, ARRAY_SIZE(mbrdone_step));
}

static int opal_lock_unlock(struct opal_dev *dev,
Expand All @@ -2119,17 +2104,16 @@ static int opal_take_ownership(struct opal_dev *dev, struct opal_key *opal)
{ end_opal_session, },
{ start_SIDASP_opal_session, opal },
{ set_sid_cpin_pin, opal },
{ end_opal_session, },
{ NULL, }
{ end_opal_session, }
};
int ret;

if (!dev)
return -ENODEV;

mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, owner_steps);
ret = next(dev);
setup_opal_dev(dev);
ret = next(dev, owner_steps, ARRAY_SIZE(owner_steps));
mutex_unlock(&dev->dev_lock);
return ret;
}
Expand All @@ -2142,17 +2126,16 @@ static int opal_activate_lsp(struct opal_dev *dev,
{ start_SIDASP_opal_session, &opal_lr_act->key },
{ get_lsp_lifecycle, },
{ activate_lsp, opal_lr_act },
{ end_opal_session, },
{ NULL, }
{ end_opal_session, }
};
int ret;

if (!opal_lr_act->num_lrs || opal_lr_act->num_lrs > OPAL_MAX_LRS)
return -EINVAL;

mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, active_steps);
ret = next(dev);
setup_opal_dev(dev);
ret = next(dev, active_steps, ARRAY_SIZE(active_steps));
mutex_unlock(&dev->dev_lock);
return ret;
}
Expand All @@ -2164,14 +2147,13 @@ static int opal_setup_locking_range(struct opal_dev *dev,
{ opal_discovery0, },
{ start_auth_opal_session, &opal_lrs->session },
{ setup_locking_range, opal_lrs },
{ end_opal_session, },
{ NULL, }
{ end_opal_session, }
};
int ret;

mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, lr_steps);
ret = next(dev);
setup_opal_dev(dev);
ret = next(dev, lr_steps, ARRAY_SIZE(lr_steps));
mutex_unlock(&dev->dev_lock);
return ret;
}
Expand All @@ -2182,8 +2164,7 @@ static int opal_set_new_pw(struct opal_dev *dev, struct opal_new_pw *opal_pw)
{ opal_discovery0, },
{ start_auth_opal_session, &opal_pw->session },
{ set_new_pw, &opal_pw->new_user_pw },
{ end_opal_session, },
{ NULL }
{ end_opal_session, }
};
int ret;

Expand All @@ -2194,8 +2175,8 @@ static int opal_set_new_pw(struct opal_dev *dev, struct opal_new_pw *opal_pw)
return -EINVAL;

mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, pw_steps);
ret = next(dev);
setup_opal_dev(dev);
ret = next(dev, pw_steps, ARRAY_SIZE(pw_steps));
mutex_unlock(&dev->dev_lock);
return ret;
}
Expand All @@ -2207,8 +2188,7 @@ static int opal_activate_user(struct opal_dev *dev,
{ opal_discovery0, },
{ start_admin1LSP_opal_session, &opal_session->opal_key },
{ internal_activate_user, opal_session },
{ end_opal_session, },
{ NULL, }
{ end_opal_session, }
};
int ret;

Expand All @@ -2220,8 +2200,8 @@ static int opal_activate_user(struct opal_dev *dev,
}

mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, act_steps);
ret = next(dev);
setup_opal_dev(dev);
ret = next(dev, act_steps, ARRAY_SIZE(act_steps));
mutex_unlock(&dev->dev_lock);
return ret;
}
Expand All @@ -2238,7 +2218,7 @@ bool opal_unlock_from_suspend(struct opal_dev *dev)
return false;

mutex_lock(&dev->dev_lock);
setup_opal_dev(dev, NULL);
setup_opal_dev(dev);

list_for_each_entry(suspend, &dev->unlk_lst, node) {
dev->tsn = 0;
Expand Down

0 comments on commit 3db8723

Please sign in to comment.