cc'ing the SCSI list, as it doesn't look like anything is making it through target-devel nowadays... https://patchwork.kernel.org/patch/10448059/ Cheers, David On Tue, 5 Jun 2018 12:00:25 +0200, David Disseldorp wrote: > SPC5r17 states that the contents of the ADDITIONAL LENGTH field are not > altered based on the allocation length, so always calculate and pack the > full key list length even if the list itself is truncated. > > According to Maged: > Yes it fixes the "Storage Spaces Persistent Reservation" test in the > Windows 2016 Server Failover Cluster validation suites when having > many connections that result in more than 8 registrations. I tested > your patch on 4.17 with iblock. > > This behaviour can be tested using the libiscsi PrinReadKeys.Truncate > test. > > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: David Disseldorp <ddiss@xxxxxxx> > Reviewed-by: Mike Christie <mchristi@xxxxxxxxxx> > Tested-by: Maged Mokhtar <mmokhtar@xxxxxxxxxxx> > --- > drivers/target/target_core_pr.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > Changes since v1: > * CC stable > * mention Maged's Windows PR test fix comment in commit message > * add Reviewed-by and Tested-by tags > > diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c > index 01ac306131c1..2e865fdaa362 100644 > --- a/drivers/target/target_core_pr.c > +++ b/drivers/target/target_core_pr.c > @@ -3727,11 +3727,16 @@ core_scsi3_pri_read_keys(struct se_cmd *cmd) > * Check for overflow of 8byte PRI READ_KEYS payload and > * next reservation key list descriptor. > */ > - if ((add_len + 8) > (cmd->data_length - 8)) > - break; > - > - put_unaligned_be64(pr_reg->pr_res_key, &buf[off]); > - off += 8; > + if ((off + 8) <= cmd->data_length) { > + put_unaligned_be64(pr_reg->pr_res_key, &buf[off]); > + off += 8; > + } > + /* > + * SPC5r17: 6.16.2 READ KEYS service action > + * The ADDITIONAL LENGTH field indicates the number of bytes in > + * the Reservation key list. The contents of the ADDITIONAL > + * LENGTH field are not altered based on the allocation length > + */ > add_len += 8; > } > spin_unlock(&dev->t10_pr.registration_lock);
