Re: [PATCH 3/3] tgt: fix scsi command leak

[Douglas Gilbert]

FUJITA Tomonori wrote:
> The failure to map user-space pages leads to scsi command leak. It can
> happens mostly because of user-space daemon bugs (or OOM). This patch
> makes tgt just notify a LLD of the failure with sense when
> blk_rq_map_user() fails.
> 
> Signed-off-by: FUJITA Tomonori <[EMAIL PROTECTED]>
> Signed-off-by: Mike Christie <[EMAIL PROTECTED]>
> ---
>  drivers/scsi/scsi_tgt_lib.c |   23 ---
>  1 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_tgt_lib.c b/drivers/scsi/scsi_tgt_lib.c
> index dc8781a..c05dff9 100644
> --- a/drivers/scsi/scsi_tgt_lib.c
> +++ b/drivers/scsi/scsi_tgt_lib.c
> @@ -459,6 +459,16 @@ static struct request *tgt_cmd_hash_look
>   return rq;
>  }
>  
> +static void scsi_tgt_build_sense(unsigned char *sense_buffer, unsigned char 
> key,
> +  unsigned char asc, unsigned char asq)
> +{
> + sense_buffer[0] = 0x70;
> + sense_buffer[2] = key;
> + sense_buffer[7] = 0xa;
> + sense_buffer[12] = asc;
> + sense_buffer[13] = asq;
> +}
> +

Tomo,
Perhaps you could add a memset(sense_buffer, 0, 18) before
those assignments and state that this is "fixed" sense
buffer format.

What about an option for descriptor sense format? With SAT now
a standard, we now have one more reason to support
descriptor format when required. The ATA PASS-THROUGH SCSI
commands in SAT use descriptor sense format to return
ATA registers.



While on the subject of sense data, I note that the
ATA folks (t13.org) are proposing an "ATA REQUEST
SENSE" command to leverage of existing SCSI
sense_key, asc, ascq tuples.

Doug Gilbert


>  int scsi_tgt_kspace_exec(int host_no, int result, u64 tag,
>unsigned long uaddr, u32 len, unsigned long 
> sense_uaddr,
>u32 sense_len, u8 rw)
> @@ -514,9 +524,16 @@ int scsi_tgt_kspace_exec(int host_no, in
>   if (len) {
>   err = scsi_map_user_pages(rq->end_io_data, cmd, uaddr, len, rw);
>   if (err) {
> - eprintk("%p %d\n", cmd, err);
> - err = -EAGAIN;
> - goto done;
> + /*
> +  * user-space daemon bugs or OOM
> +  * TODO: we can do better for OOM.
> +  */
> + eprintk("cmd %p ret %d uaddr %lx len %d rw %d\n",
> + cmd, err, uaddr, len, rw);
> + cmd->result = SAM_STAT_CHECK_CONDITION;
> + memset(cmd->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE);
> + scsi_tgt_build_sense(cmd->sense_buffer,
> +  HARDWARE_ERROR, 0, 0);
>   }
>   }
>   err = scsi_tgt_transfer_response(cmd);

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Possible bug in scsi_lib.c:scsi_req_map_sg()

[Mike Christie]

Dachepalli, Sudhir wrote:
> Where is the depricated warning that you mentioned about ?
> I tried to look in scsi_lib.c and scsi_device.h
> 

I meant in the first versions of the patches there was a warning. Did
you also try the workarounds mentioned in the bugzilla, to allocate
memory like sg and st?
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: Possible bug in scsi_lib.c:scsi_req_map_sg()

[Dachepalli, Sudhir]

Mike ,

Our driver gets called in with the following fashion through the
queuecommand.

scsi_request_fn() -> scsi_dispatch_cmd() -> rtn =
host->hostt->queuecommand(cmd, scsi_done);

We are using the "cmd" ( scsi_cmnd) as a pass through with out touching
the "request_buffer" and "request_bufflen".

We do not allocate memory similar to sg or st for page allocations.

The request_buffer should already contain the scatter gather list built.


Regards,
Sudhir 

-Original Message-
From: Mike Christie [mailto:[EMAIL PROTECTED] 
Sent: Saturday, March 03, 2007 6:04 PM
To: Dachepalli, Sudhir
Cc: Benny Halevy; Jens Axboe; Boaz Harrosh; linux-scsi@vger.kernel.org;
James Bottomley
Subject: Re: Possible bug in scsi_lib.c:scsi_req_map_sg()

Dachepalli, Sudhir wrote:
> Where is the depricated warning that you mentioned about ?
> I tried to look in scsi_lib.c and scsi_device.h
> 

I meant in the first versions of the patches there was a warning. Did
you also try the workarounds mentioned in the bugzilla, to allocate
memory like sg and st?
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html