linux-kernel - Re: [RFC PATCH 04/10] scsi/constants: Cleanup printk message in scsi_dump_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: <1408115326.1223.147.camel@localhost.localdomain>
Date:	Fri, 15 Aug 2014 11:08:46 -0400
From:	Ewan Milne <emilne@...hat.com>
To:	Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@...achi.com>
Cc:	Hannes Reinecke <hare@...e.de>, linux-scsi@...r.kernel.org,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	yrl.pp-manager.tt@...achi.com, linux-kernel@...r.kernel.org,
	"James E.J. Bottomley" <JBottomley@...allels.com>,
	Hidehiro Kawai <hidehiro.kawai.ez@...achi.com>,
	Doug Gilbert <dgilbert@...erlog.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Christoph Hellwig <hch@....de>
Subject: Re: [RFC PATCH 04/10] scsi/constants: Cleanup printk message in
 scsi_dump_sense_buffer()

On Fri, 2014-08-08 at 11:50 +0000, Yoshihiro YUNOMAE wrote:
> Unrecognized sense data should be output after linebuf is filled because
> "[%s] Unrecognized sense data (in hex): %s" message is output many times in
> loop.
> 
> Signed-off-by: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@...achi.com>
> Cc: Hannes Reinecke <hare@...e.de>
> Cc: Doug Gilbert <dgilbert@...erlog.com>
> Cc: Martin K. Petersen <martin.petersen@...cle.com>
> Cc: Christoph Hellwig <hch@....de>
> Cc: "James E.J. Bottomley" <JBottomley@...allels.com>
> Cc: Hidehiro Kawai <hidehiro.kawai.ez@...achi.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> ---
>  drivers/scsi/constants.c |   13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
> index 5956d4d..6fad6b4 100644
> --- a/drivers/scsi/constants.c
> +++ b/drivers/scsi/constants.c
> @@ -1385,16 +1385,13 @@ EXPORT_SYMBOL(scsi_print_sense_hdr);
>  
>  static void
>  scsi_dump_sense_buffer(struct scsi_device *sdev, const char *prefix,
> -		       const unsigned char *sense_buffer, int sense_len,
> -		       struct scsi_sense_hdr *sshdr)
> +		       const unsigned char *sense_buffer, int sense_len)
>  {
>  	char linebuf[128];
>  	int i, linelen, remaining;
>  
>  	if (sense_len < 32)
>  		sense_len = 32;
> -	sdev_printk(KERN_INFO, sdev,
> -		    "[%s] Unrecognized sense data (in hex):", prefix);
>  
>  	remaining = sense_len;
>  	for (i = 0; i < sense_len; i += 16) {
> @@ -1403,9 +1400,10 @@ scsi_dump_sense_buffer(struct scsi_device *sdev, const char *prefix,
>  
>  		hex_dump_to_buffer(sense_buffer + i, linelen, 16, 1,
>  				   linebuf, sizeof(linebuf), false);
> -		sdev_printk(KERN_INFO, sdev, "[%s] Sense: %s\n",
> -			    prefix, linebuf);
>  	}
> +	sdev_printk(KERN_INFO, sdev,
> +		    "[%s] Unrecognized sense data (in hex): %s",
> +		    prefix, linebuf);
>  }

See my earlier comment regarding PATCH 03/10.

This doesn't look right -- In Hannes' tree what the code is doing is
printing out a separate line for each 16 bytes of the sense data.
Your change will cause only the last (partial?) 16 bytes to be printed.

The removal of the unused sshdr argument is fine, though.

-Ewan

>  
>  static void
> @@ -1467,8 +1465,7 @@ void __scsi_print_sense(struct scsi_device *sdev, const char *name,
>  
>  	if (!scsi_normalize_sense(sense_buffer, sense_len, &sshdr)) {
>  		/* this may be SCSI-1 sense data */
> -		scsi_dump_sense_buffer(sdev, name, sense_buffer,
> -				       sense_len, &sshdr);
> +		scsi_dump_sense_buffer(sdev, name, sense_buffer, sense_len);
>  		return;
>  	}
>  
> 
> --
> 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/


--
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/