On Tue, 6 May 2008 16:50:40 +1000
"ronnie sahlberg" <[EMAIL PROTECTED]> wrote:
> >From 37275d5cfafc87a165824cbf9722ea742a770403 Mon Sep 17 00:00:00 2001
> From: Ronnie Sahlberg <[EMAIL PROTECTED]>
> Date: Tue, 6 May 2008 16:37:07 +1000
> Subject: [PATCH] Fix a memory corruption bug in build_mode_page
>
> If an application is providing a small allocation size when asking for a large
> mode page hte memcpy() in build_mode_page can cause memory to be overwritten
> and cause a crash.
>
> Signed-off-by: Ronnie Sahlberg <[EMAIL PROTECTED]>
> ---
> usr/spc.c | 63 ++++++++++++++++++++++++++++++++++--------------------------
> 1 files changed, 36 insertions(+), 27 deletions(-)
Thanks a lot,
> diff --git a/usr/spc.c b/usr/spc.c
> index e3e4d98..35de2e1 100644
> --- a/usr/spc.c
> +++ b/usr/spc.c
> @@ -335,22 +335,28 @@ int spc_test_unit(int host_no, struct scsi_cmd *cmd)
> *
> * Returns number of bytes copied.
> */
> -static int build_mode_page(uint8_t *data, struct mode_pg *pg, int update)
> +static uint8_t *build_mode_page(uint8_t *data, uint8_t *p, int alloc_len,
> +struct mode_pg *pg)
> {
> - uint8_t *p;
> - int len;
> + int i;
> +
> + if (alloc_len > (p-data))
> + *p = pg->pcode;
> + p++;
> +
> +
> + if (alloc_len > (p-data))
> + *p = pg->pcode_size;
> + p++;
> +
>
> - len = pg->pcode_size;
> - if (update) {
> - data[0] = pg->pcode;
> - data[1] = len;
> + for (i = 0; i < pg->pcode_size; i++) {
> + if (alloc_len > (p-data))
> + *p = pg->mode_data[i];
> + p++;
> }
> - p = &data[2];
> - len += 2;
> - if (update)
> - memcpy(p, pg->mode_data, pg->pcode_size);
>
> - return len;
> + return p;
> }
>
> /**
> @@ -362,12 +368,13 @@ static int build_mode_page(uint8_t *data, struct
> mode_pg *pg, int update)
> */
> int spc_mode_sense(int host_no, struct scsi_cmd *cmd)
> {
> - int len = 0;
> uint8_t *data = NULL, *scb, mode6, dbd, pcode, subpcode;
> + uint8_t *p;
> uint16_t alloc_len;
> unsigned char key = ILLEGAL_REQUEST;
> uint16_t asc = ASC_INVALID_FIELD_IN_CDB;
> struct mode_pg *pg;
> + int i;
>
> scb = cmd->scb;
> mode6 = (scb[0] == 0x1a);
> @@ -383,10 +390,10 @@ int spc_mode_sense(int host_no, struct scsi_cmd *cmd)
>
> if (mode6) {
> alloc_len = scb[4];
> - len = 4;
> + p = &data[4];
> } else {
> alloc_len = (scb[7] << 8) + scb[8];
> - len = 8;
> + p = &data[8];
> }
> if (scsi_get_in_length(cmd) < alloc_len)
We still have the same problem here? If a buggy (or malicious)
initiator sends a bogus cdb, alloc_len can be larger than what we
actually allocated.
I'll fix this bug later.
_______________________________________________
Stgt-devel mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/stgt-devel
