Skip to contentSkip to conversation

zfcp: fix sense_buffer access bug

PATCH In review FUJITA Tomonori 4 participants 5 messages Jan 27, 2008 – Jan 28, 2008

Subsystems (3)
SCSI s390 s390 zfcp driver
lore
Share & tools
lore

FUJITA Tomonori

author
#0 lore raw

PATCH 1/1 zfcp: fix sense_buffer access bug

The commit de25deb18016f66dcdede165d07654559bb332bc changed scsi_cmnd.sense_buffer from a static array to a dynamically allocated buffer. We can't access to sense_buffer in '&cmd->sense_buffer' way.

Signed-off-by FUJITA Tomonori <[email protected]>
---
 drivers/s390/scsi/zfcp_fsf.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/s390/scsi/zfcp_fsf.c +2 -2
index fe57941..a9a147d 100644
@@ -4224,10 +4224,10 @@ zfcp_fsf_send_fcp_command_task_handler(struct zfcp_fsf_req *fsf_req)
 
 		ZFCP_LOG_TRACE("%i bytes sense data provided by FCP\n",
 			       fcp_rsp_iu->fcp_sns_len);
-		memcpy(&scpnt->sense_buffer,
+		memcpy(scpnt->sense_buffer,
 		       zfcp_get_fcp_sns_info_ptr(fcp_rsp_iu), sns_len);
 		ZFCP_HEX_DUMP(ZFCP_LOG_LEVEL_TRACE,
-			      (void *) &scpnt->sense_buffer, sns_len);
+			      (void *)scpnt->sense_buffer, sns_len);
 	}
 
 	/* check for overrun */
Signature · 1 line
1.5.3.4
#1 lore raw
Quoted 16 lines · FUJITA Tomonori

The commit de25deb18016f66dcdede165d07654559bb332bc changed scsi_cmnd.sense_buffer from a static array to a dynamically allocated buffer. We can't access to sense_buffer in '&cmd->sense_buffer' way.

Signed-off-by FUJITA Tomonori <[email protected]>
---
 drivers/s390/scsi/zfcp_fsf.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/s390/scsi/zfcp_fsf.c +0 -0
index fe57941..a9a147d 100644
@@ -4224,10 +4224,10 @@ zfcp_fsf_send_fcp_command_task_handler(struct zfcp_fsf_req *fsf_req)
 
 		ZFCP_LOG_TRACE("%i bytes sense data provided by FCP\n",

drivers/s390/scsi/zfcp_fsf.c zfcp_fsf_send_fcp_command_task_handler(struct zfcp_fsf_req *fsf_req) · line 4233

Quoted 10 lines
drivers/s390/scsi/zfcp_fsf.c +2 -2
 			       fcp_rsp_iu->fcp_sns_len);
-		memcpy(&scpnt->sense_buffer,
+		memcpy(scpnt->sense_buffer,
 		       zfcp_get_fcp_sns_info_ptr(fcp_rsp_iu), sns_len);
 		ZFCP_HEX_DUMP(ZFCP_LOG_LEVEL_TRACE,
-			      (void *) &scpnt->sense_buffer, sns_len);
+			      (void *)scpnt->sense_buffer, sns_len);
 	}
 
 	/* check for overrun */

ACK for fixing the access to the sense buffer.

We are working internally on cleaning up the zfcp messages. With this change, the 'trace' and 'hex dump' messages will disappear. So, could you simply remove the ZFCP_HEX_DUMP message above, instead of fixing it?

Christof

#2 lore raw
Quoted 26 lines · On Mon, 28 Jan 2008 08:46:25 +0100 Christof Schmitt…
Quoted 25 lines · FUJITA Tomonori

The commit de25deb18016f66dcdede165d07654559bb332bc changed scsi_cmnd.sense_buffer from a static array to a dynamically allocated buffer. We can't access to sense_buffer in '&cmd->sense_buffer' way.

Signed-off-by FUJITA Tomonori <[email protected]>
---
 drivers/s390/scsi/zfcp_fsf.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/s390/scsi/zfcp_fsf.c +2 -2
index fe57941..a9a147d 100644
@@ -4224,10 +4224,10 @@ zfcp_fsf_send_fcp_command_task_handler(struct zfcp_fsf_req *fsf_req)
 
 		ZFCP_LOG_TRACE("%i bytes sense data provided by FCP\n",
 			       fcp_rsp_iu->fcp_sns_len);
-		memcpy(&scpnt->sense_buffer,
+		memcpy(scpnt->sense_buffer,
 		       zfcp_get_fcp_sns_info_ptr(fcp_rsp_iu), sns_len);
 		ZFCP_HEX_DUMP(ZFCP_LOG_LEVEL_TRACE,
-			      (void *) &scpnt->sense_buffer, sns_len);
+			      (void *)scpnt->sense_buffer, sns_len);
 	}
Quoted 8 lines

drivers/s390/scsi/zfcp_fsf.c zfcp_fsf_send_fcp_command_task_handler(struct zfcp_fsf_req *fsf_req) · line 4233

Quoted 1 line
drivers/s390/scsi/zfcp_fsf.c +0 -0
 	/* check for overrun */

ACK for fixing the access to the sense buffer.

We are working internally on cleaning up the zfcp messages. With this change, the 'trace' and 'hex dump' messages will disappear. So, could you simply remove the ZFCP_HEX_DUMP message above, instead of fixing it?

I can but James has already merged the above patch to scsi-misc. So it would be more convenient for everyone if you could rebase your patchset on top of scsi-misc?

#3 lore raw
Quoted 10 lines · FUJITA Tomonori
Quoted 6 lines

ACK for fixing the access to the sense buffer.

We are working internally on cleaning up the zfcp messages. With this change, the 'trace' and 'hex dump' messages will disappear. So, could you simply remove the ZFCP_HEX_DUMP message above, instead of fixing it?

I can but James has already merged the above patch to scsi-misc. So it would be more convenient for everyone if you could rebase your patchset on top of scsi-misc?

Ok, if the pach is already merged, i am fine with that. We will have to rebase the patches we are working on anyway. Thanks for providing the fix.

Christof

maintainer
#4 lore raw
Quoted 8 lines · Christof Schmitt
Quoted 7 lines · FUJITA Tomonori
Quoted 6 lines

ACK for fixing the access to the sense buffer.

We are working internally on cleaning up the zfcp messages. With this change, the 'trace' and 'hex dump' messages will disappear. So, could you simply remove the ZFCP_HEX_DUMP message above, instead of fixing it?

Quoted 7 lines
Quoted 3 lines

I can but James has already merged the above patch to scsi-misc. So it would be more convenient for everyone if you could rebase your patchset on top of scsi-misc?

Ok, if the pach is already merged, i am fine with that. We will have to rebase the patches we are working on anyway. Thanks for providing the fix.

It's in my upstream tree, but not merged with Linus and I can easily rebase. However, it is a critical bug fix, without it the driver will likely oops from overwriting command structures, so I'd rather keep it separate from any cleanup patches.

James