Re: [PATCH] libata: Avoid overflow in ata_tf_read_block() when tf->hba_lbal > 127

["Phillip O'Donnell" <phillip.odonnell@gmail.com>]

Thanks Roland,

Right now, my observations indicate that my original fault occurs when
a command (of any type, e.g, read or flush ... ) times out, which
should trigger the sense routines. Next thing I need to do is add some
debug code to identify whether this only occurs if the timeout happens
on a sector above 2^31.

I've identified a reasonably reliable testcase for my fault, so I'll
add that patch and see if it occurs.

I'll let you know how it pans out.

Cheers,
Phillip

On Wed, Nov 5, 2008 at 7:34 AM, Roland Dreier <rdreier@cisco.com> wrote:
>
> Phillip O'Donnell <phillip.odonnell@gmail.com> pointed out that the same
> sign extension bug that was fixed in commit ba14a9c2 ("libata: Avoid
> overflow in ata_tf_to_lba48() when tf->hba_lbal > 127") also appears to
> exist in ata_tf_read_block().  Fix this by adding a cast to u64.
>
> Signed-off-by: Roland Dreier <rolandd@cisco.com>
> ---
> I don't have any way to test this -- I guess you would have to get an
> error on a block above 2G (ie data above 1TB)?  But it looks "obviously
> correct" enough to add to -next I guess.
>
>  drivers/ata/libata-core.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 622350d..a6ad862 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -612,7 +612,7 @@ u64 ata_tf_read_block(struct ata_taskfile *tf, struct ata_device *dev)
>                if (tf->flags & ATA_TFLAG_LBA48) {
>                        block |= (u64)tf->hob_lbah << 40;
>                        block |= (u64)tf->hob_lbam << 32;
> -                       block |= tf->hob_lbal << 24;
> +                       block |= (u64)tf->hob_lbal << 24;
>                } else
>                        block |= (tf->device & 0xf) << 24;
>