On Thu, 2017-05-11 at 18:39 +0000, Bart Van Assche wrote: > On Thu, 2017-05-11 at 10:21 -0700, Nicholas A. Bellinger wrote: > > This reverts commit 0e2eb7d12eaa8e391bf5615d4271bb87a649caaa > > > > Author: Bart Van Assche <bart.vanassche@xxxxxxxxxxx> > > Date: Thu Mar 30 10:12:39 2017 -0700 > > > > target: Fix VERIFY and WRITE VERIFY command parsing > > > > This patch broke existing behaviour for WRITE_VERIFY because > > it dropped the original SCF_SCSI_DATA_CDB assignment for > > bytchk = 0 so target_cmd_size_check() no longer rejected > > this case, allowing an overflow case to trigger an OOPs > > in iscsi-target. > > > > Since the short term and long term fixes are still being > > discussed, revert it for now since it's late in the merge > > window and try again in v4.13-rc1. > > Hello Nic, > > In transport_generic_new_cmd(), called from iscsit_process_scsi_cmd() to > submit an iSCSI command to the LIO core, one can see that target_alloc_sgls() > is called whether or not SCF_SCSI_DATA_CDB has been set. Wrong. iscsit_process_scsi_cmd() returns '1' to signal dump_payload to iscsit_get_immediate_data() when any CDB with SCF_SCSI_DATA_CDB is set. > So I don't think > that what you wrote in the patch description is correct. If you have a look > at my v2 patch series you will see that the buffer overflow I reported can > be triggered for any SCSI command and not just for VERIFY / WRITE AND VERIFY. > All that's needed to trigger a buffer overflow with the iSCSI target driver > is to send about 4 * PAGE_SIZE bytes more immediate data than the Data-Out > buffer size specified through the CDB. Considering a full page is still allocated for non SCF_SCSI_DATA_CDB, and iscsit_map_iovec() still checks for SGL array overflow, I don't think you'll be able to trigger anything. If you can, you should post the test case to prove it. :) Regardless, I'm going to push the change post -rc1 to address the regression that Roland's v4.3.y change introduced wrt to control WRITE CDB overflow: http://marc.info/?l=linux-scsi&m=149452576930990&w=2 Like I said earlier, you are more than welcome to add full WRITE CDB overflow support, but since it's a new feature it's not v4.12 material. -- 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
