> On 15-02-24 04:37 PM, Tomas Winkler wrote: > > 1. Fix the error check: the alloc length should be > 16 > > and not > 4 > > You are proposing to make a marginally incorrect test > completely incorrect! Quoting from the spec: The ALLOCATION LENGTH field is defined in 2.2.6. The allocation length should be at least 16. Note. Device servers compliant with SPC return CHECK CONDITION status, with the sense key set to ILLEGAL REQUEST, and the additional sense code set to INVALID FIELD IN CDB when the allocation length is less than 16 bytes. This is how it is implemented also in other drivers in the scsi drivers. > > The governing rule for almost all "allocation length" fields > in SCSI commands (that return data-in) is: > > "An allocation length of zero specifies that no data shall > be transferred. This condition shall not be considered an > error." [in section 4.2.5.6 of spc5r03.pdf and REPORT LUNS > refers to this] Correct, but this wasn't the case also in the original code it checked for < 4. > If the REPORT LUNS allocation length is less that 4 then > the caller doesn't even get the response length back so that > has no practical use IMO. If allocation_length=0 and the SCSI > status is GOOD then all that tells you is that REPORT LUNS is > supported, but it has been a mandatory command for 10 years > so that is to be expected. I believe that also original code doesn't handle that case. > > 2. Remove duplicated boundary checks which simplify > > the fill-in loop > > 3. Use more of scsi generic API > > Something else you might fix is the stack allocation of > 2048+8 bytes: > u8 arr[SDEBUG_RLUN_ARR_SZ]; > > It would be better to work out how many LUs the > parent target has and use kmalloc() (or friend) > to make a response buffer large enough. That might be fixed the question is if this not out of scope for emulation purposes. To get spec limit to 16K maybe it will overcomplicate code uncessary. And > "large enough" needs to be no larger than the > allocation_length indicates however that requires > extra care in the loop stopping conditions. We are not getting anything unexpected insde the loop, the boundaries are known before we entering the loop. Tomas -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
