linux-kernel - Re: [PATCH 07/14] ide-cd: clear sense buffer before issuing request sense

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090326072044.GA32155@liondog.tnic>
Date:	Thu, 26 Mar 2009 08:20:44 +0100
From:	Borislav Petkov <petkovbb@...glemail.com>
To:	Tejun Heo <tj@...nel.org>
Cc:	bzolnier@...il.com, linux-kernel@...r.kernel.org, axboe@...nel.dk,
	linux-ide@...r.kernel.org
Subject: Re: [PATCH 07/14] ide-cd: clear sense buffer before issuing
	request sense

Hi Tejun,

On Wed, Mar 25, 2009 at 01:06:09AM +0900, Tejun Heo wrote:
> Impact: code simplification
> 
> ide_cd_request_sense_fixup() clears the tail of the sense buffer if
> the device didn't completely fill it.  This patch makes
> cdrom_queue_request_sense() clear the sense buffer before issuing the
> command instead of clearing it afterwards.  This simplifies code and
> eases future changes.
> 
> Signed-off-by: Tejun Heo <tj@...nel.org>
> ---
>  drivers/ide/ide-cd.c |   12 ++++--------
>  1 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
> index 35729a4..b3736b4 100644
> --- a/drivers/ide/ide-cd.c
> +++ b/drivers/ide/ide-cd.c
> @@ -217,6 +217,8 @@ static void cdrom_queue_request_sense(ide_drive_t *drive, void *sense,
>  	if (sense == NULL)
>  		sense = &info->sense_data;
>  
> +	memset(sense, 0, 18);
> +

Hmm, not quite. If its info->sense_data, then 18 is not correct. Rather,
sizeof(struct request_sense) would be more likely the proper length. So,
actually, it would be more like:

unsigned sense_len = 18;

if (sense == NULL) {
	sense = &info->sense_data;
	sense_len = sizeof(struct request_sense);
}

memset(sense, 0, sense_len);

>  	/* stuff the sense request in front of our current request */
>  	blk_rq_init(NULL, rq);
>  	rq->cmd_type = REQ_TYPE_ATA_PC;
> @@ -520,14 +522,8 @@ static void ide_cd_request_sense_fixup(ide_drive_t *drive, struct ide_cmd *cmd)
>  	 * and some drives don't send them.  Sigh.
>  	 */
>  	if (rq->cmd[0] == GPCMD_REQUEST_SENSE &&
> -	    cmd->nleft > 0 && cmd->nleft <= 5) {
> -		unsigned int ofs = cmd->nbytes - cmd->nleft;
> -
> -		while (cmd->nleft > 0) {
> -			*((u8 *)rq->data + ofs++) = 0;
> -			cmd->nleft--;
> -		}
> -	}
> +	    cmd->nleft > 0 && cmd->nleft <= 5)
> +		cmd->nleft = 0;
>  }
>

Otherwise, I like it.

-- 
Regards/Gruss,
    Boris.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/