Hi Sebastian,
Apologies for the delay here. Still catching up on a bunch of holiday
items..
On Tue, 2012-12-18 at 16:11 +0100, Sebastian Andrzej Siewior wrote:
> I am not sure I know what I do. If cmd->data_length 4 then it does not
> run into the "SET TARGET PORT GROUPS parameter list length too small"
> error. Later ptr is assigned to &buf[4] which is only valid if length is
> atleast 5 rigtht? Anyway, we never enter the while loop because "4 < 4"
> not true and we leave with rc undefined.
> Looking at the while loop, it reads ptr + 2 but we checked the size
> until ptr so I am not sure whether this is correct or not.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
> drivers/target/target_core_alua.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
> index 8ddab35..92f8eb1 100644
> --- a/drivers/target/target_core_alua.c
> +++ b/drivers/target/target_core_alua.c
> @@ -220,7 +220,7 @@ target_emulate_set_target_port_groups(struct se_cmd *cmd)
> if (!l_port)
> return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
>
> - if (cmd->data_length < 4) {
> + if (cmd->data_length <= 4) {
> pr_warn("SET TARGET PORT GROUPS parameter list length %u too"
> " small\n", cmd->data_length);
> return TCM_INVALID_PARAMETER_LIST;
Mmmm, I think this check is still incorrect for the zero-length case..
According to SPC-4, SET TARGET PORT GROUPS this is allowed:
"The PARAMETER LIST LENGTH field specifies the length in byes of the
target port group management parameters that shall be transferred from
the application client to the device server. A parameter list length of
zero specifies that no data shall be transferred, and that no change
shall be made in the target port asymmetric access state of
any target port groups or target ports. If the parameter list length
violates the vendor specific length requirements, the command shall be
terminated with CHECK CONDITION status, with the sense key set to
ILLEGAL REQUEST, and the additional sense code set to INVALID FIELD IN
CDB."
Btw there are a number of zero-length CDB regressions in v3.8-rc from
the sense_reason_t conversion that I'm working to resolve, and will plan
to verify both cases here.
Thanks for reporting!
--nab
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
