The reservation key checking in do_mpath_persistent_reserve_out() was slightly wrong. It allowed invalid keys for preempting. It now correctly checks the reservation key for the preempt commands.
It also was a little overly strict in some places. Formerly, it only allowed registering from any key to the configured key or registering from the configured key to any key (as long as you use the prkeys file). Now it also allows unregistering from any key and registering an unregistered device to any key (as long as you use the prkeys file). Also, clarify the code by replacing prkey with a bool tracking if you are registering or unregistering. Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com> --- libmpathpersist/mpath_persist_int.c | 52 ++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c index c6b2e788..71933fe5 100644 --- a/libmpathpersist/mpath_persist_int.c +++ b/libmpathpersist/mpath_persist_int.c @@ -709,9 +709,9 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, { struct multipath *mpp; int ret; - uint64_t prkey; + uint64_t zerokey = 0; struct config *conf; - bool preempting_reservation = false; + bool unregistering, preempting_reservation = false; ret = mpath_get_map(curmp, pathvec, fd, &mpp); if (ret != MPATH_PR_SUCCESS) @@ -727,11 +727,12 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, if (rq_servact == MPATH_PROUT_REG_IGN_SA) set_ignored_key(mpp, paramp->key); - memcpy(&prkey, paramp->sa_key, 8); - if (mpp->prkey_source == PRKEY_SOURCE_FILE && prkey && + unregistering = (memcmp(&zerokey, paramp->sa_key, 8) == 0); + if (mpp->prkey_source == PRKEY_SOURCE_FILE && !unregistering && (rq_servact == MPATH_PROUT_REG_IGN_SA || (rq_servact == MPATH_PROUT_REG_SA && (!get_be64(mpp->reservation_key) || + memcmp(paramp->key, &zerokey, 8) == 0 || memcmp(paramp->key, &mpp->reservation_key, 8) == 0)))) { memcpy(&mpp->reservation_key, paramp->sa_key, 8); if (update_prkey_flags(mpp->alias, get_be64(mpp->reservation_key), @@ -742,19 +743,38 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, } } - if (!get_be64(mpp->reservation_key) && - (prkey || rq_servact != MPATH_PROUT_REG_IGN_SA)) { - condlog(0, "%s: no configured reservation key", mpp->alias); - return MPATH_PR_SYNTAX_ERROR; + /* + * If you are registering a non-zero key, mpp->reservation_key + * must be set and must equal paramp->sa_key. + * If you're not registering a key, mpp->reservation_key must be + * set, and must equal paramp->key + */ + if ((rq_servact == MPATH_PROUT_REG_IGN_SA || + rq_servact == MPATH_PROUT_REG_SA)) { + if (!unregistering && !get_be64(mpp->reservation_key)) { + condlog(0, "%s: no configured reservation key", + mpp->alias); + return MPATH_PR_SYNTAX_ERROR; + } + if (!unregistering && + memcmp(paramp->sa_key, &mpp->reservation_key, 8)) { + condlog(0, "%s: configured reservation key doesn't match: 0x%" PRIx64, + mpp->alias, get_be64(mpp->reservation_key)); + return MPATH_PR_SYNTAX_ERROR; + } + } else { + if (!get_be64(mpp->reservation_key)) { + condlog(0, "%s: no configured reservation key", + mpp->alias); + return MPATH_PR_SYNTAX_ERROR; + } + if (memcmp(paramp->key, &mpp->reservation_key, 8)) { + condlog(0, "%s: configured reservation key doesn't match: 0x%" PRIx64, + mpp->alias, get_be64(mpp->reservation_key)); + return MPATH_PR_SYNTAX_ERROR; + } } - if (memcmp(paramp->key, &mpp->reservation_key, 8) && - memcmp(paramp->sa_key, &mpp->reservation_key, 8) && - (prkey || rq_servact != MPATH_PROUT_REG_IGN_SA)) { - condlog(0, "%s: configured reservation key doesn't match: 0x%" PRIx64, - mpp->alias, get_be64(mpp->reservation_key)); - return MPATH_PR_SYNTAX_ERROR; - } switch(rq_servact) { @@ -785,7 +805,7 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, switch (rq_servact) { case MPATH_PROUT_REG_SA: case MPATH_PROUT_REG_IGN_SA: - if (prkey == 0) { + if (unregistering) { update_prflag(mpp->alias, 0); update_prkey(mpp->alias, 0); } else -- 2.48.1