linux-kernel - Re: [PATCH rc8-mm1] hotfix libata-scsi corruption

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: <479652D2.10800@garzik.org>
Date:	Tue, 22 Jan 2008 15:32:18 -0500
From:	Jeff Garzik <jeff@...zik.org>
To:	James Bottomley <James.Bottomley@...senPartnership.com>
CC:	Hugh Dickins <hugh@...itas.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Alan Stern <stern@...land.harvard.edu>,
	linux-kernel@...r.kernel.org, linux-ide@...r.kernel.org,
	linux-scsi <linux-scsi@...r.kernel.org>
Subject: Re: [PATCH rc8-mm1] hotfix libata-scsi corruption

James Bottomley wrote:
> Actually, I don't think it's a smaller I/O issue.  The SMART protocol
> specifically mandates that the transfers for SMART READ DATA and SMART
> READ LOG shall be 512 bytes).  However, the pio transfer routine does
> seem to be assuming sector alignment as well, which will be where your
> problems are coming from.  I think we need to specify sector minimum
> alignment for ata (but not atapi, which has its own non sector size pio
> routine).  How about the attached?
> 
> We have to do this for all ATA devices, because they'll likely all
> support SMART, and SMART is defined to be a PIO command.
> 
> James
> 
> ---
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 4bb268b..bc5cf6b 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -824,9 +824,6 @@ static void ata_scsi_sdev_config(struct scsi_device *sdev)
>  	 * requests.
>  	 */
>  	sdev->max_device_blocked = 1;
> -
> -	/* set the min alignment */
> -	blk_queue_update_dma_alignment(sdev->request_queue, ATA_DMA_PAD_SZ - 1);
>  }
>  
>  static void ata_scsi_dev_config(struct scsi_device *sdev,
> @@ -842,7 +839,14 @@ static void ata_scsi_dev_config(struct scsi_device *sdev,
>  	if (dev->class == ATA_DEV_ATAPI) {
>  		struct request_queue *q = sdev->request_queue;
>  		blk_queue_max_hw_segments(q, q->max_hw_segments - 1);
> -	}
> +
> +		/* set the min alignment */
> +		blk_queue_update_dma_alignment(sdev->request_queue,
> +					       ATA_DMA_PAD_SZ - 1);
> +	} else
> +		/* ATA devices must be sector aligned */
> +		blk_queue_update_dma_alignment(sdev->request_queue,
> +					       ATA_SECT_SIZE - 1);
>  
>  	if (dev->flags & ATA_DFLAG_AN)
>  		set_bit(SDEV_EVT_MEDIA_CHANGE, sdev->supported_events);


ACK

Unlike ATAPI, ATA is indeed all 512-byte alignment transfers (_not_ 
sector size, which may or may not be 512 bytes)

Does this apply to libata?  libata + jejb dma alignment patch?

What tree...

	Jeff



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