On 12/9/22 4:40 AM, John Garry wrote: >> * head injection*required* here otherwise quiesce won't work >> @@ -249,13 +238,14 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, >> if (unlikely(scmd->resid_len > 0 && scmd->resid_len <= bufflen)) >> memset(buffer + bufflen - scmd->resid_len, 0, scmd->resid_len); >> - if (resid) >> - *resid = scmd->resid_len; >> - if (sense && scmd->sense_len) >> - memcpy(sense, scmd->sense_buffer, SCSI_SENSE_BUFFERSIZE); >> - if (sshdr) >> - scsi_normalize_sense(scmd->sense_buffer, scmd->sense_len, >> - sshdr); >> + if (args.resid) >> + *args.resid = scmd->resid_len; >> + if (args.sense && scmd->sense_len) > > I am not sure that you require the sense_len check as you effectively have that same check in scsi_execute_args(), which is the only caller which would have args.sense set. But I suppose __scsi_execute() is still a public API (so should still check); but, by that same token, we have no sanity check for args.sense_len value here then. Is it possible to make __scsi_execute() non-public or move/add the check for proper sense_len here? I'm being extra cautious about this, I suppose. Do people want the BUILD_BUG_ON we have now or a WARN/BUG? If we move to a single check in __scsi_execute or some non-macro wrapper around it then we have to do a WARN/BUG. We do the macro approach now so we can do the BUILD_BUG_ON.
