| 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
| ||
|
Date: Mon, 1 Mar 2010 14:13:35 -0800
From: Andrew Morton <akpm@...ux-foundation.org>
To: "Daniel Taylor" <Daniel.Taylor@....com>
Cc: <linux-kernel@...r.kernel.org>,
OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
Subject: Re: [PATCH] msdos: add support for large disks
On Wed, 24 Feb 2010 19:17:47 -0800
"Daniel Taylor" <Daniel.Taylor@....com> wrote:
> In order to use disks larger than 2TiB on Windows XP, it is necessary to use
> 4096-byte logical sectors in an MBR.
>
> Although the kernel storage and functions called from msdos.c used
> "sector_t" internally, msdos.c still used u32 variables, which results in
> the ability to handle XP-compatible large disks.
>
> This patch changes the internal variables to "sector_t".
>
Please Cc OGAWA Hirofumi <hirofumi@...l.parknet.co.jp> and myself on
this work.
The patch was really badly wordwrapped. Please fix up your email
client for next time.
> ---
> diff --git a/fs/partitions/msdos.c b/fs/partitions/msdos.c
> index 0028d2e..fa9c77d 100644
> --- a/fs/partitions/msdos.c
> +++ b/fs/partitions/msdos.c
> @@ -104,12 +104,12 @@ static int aix_magic_present(unsigned char *p, struct
> block_device *bdev)
>
> static void
> parse_extended(struct parsed_partitions *state, struct block_device *bdev,
> - u32 first_sector, u32 first_size)
> + sector_t first_sector, sector_t first_size)
> {
> struct partition *p;
> Sector sect;
> unsigned char *data;
> - u32 this_sector, this_size;
> + sector_t this_sector, this_size;
> int sector_size = bdev_logical_block_size(bdev) / 512;
> int loopct = 0; /* number of links followed
> without finding a data partition */
> @@ -145,15 +145,16 @@ parse_extended(struct parsed_partitions *state, struct
> block_device *bdev,
> * First process the data partition(s)
> */
> for (i=0; i<4; i++, p++) {
> - u32 offs, size, next;
> + sector_t offs, size, next;
> if (!NR_SECTS(p) || is_extended_partition(p))
> continue;
>
> /* Check the 3rd and 4th entries -
> these sometimes contain random garbage */
> - offs = START_SECT(p)*sector_size;
> - size = NR_SECTS(p)*sector_size;
> + offs =
> (sector_t)(START_SECT(p))*(sector_t)(sector_size);
> + size =
> (sector_t)(NR_SECTS(p))*(sector_t)(sector_size);
The patch would be cleaner if START_SECT() and NR_SECTS() were
returning the correct type. I suggest converting these to regular C
functions, renaming them to start_sect() and nr_sects() (or something
more descriptive if you like) and making them return a sector_t.
Making local variable `sector_size' have type sector_t would eliminate
the other typecast and is a logical enough thing to do, I think.
> next = this_sector + offs;
> +
> if (i >= 2) {
> if (offs + size > this_size)
> continue;
>
> ...
>
> @@ -263,14 +264,14 @@ parse_bsd(struct parsed_partitions *state, struct
> block_device *bdev,
> if (le16_to_cpu(l->d_npartitions) < max_partitions)
> max_partitions = le16_to_cpu(l->d_npartitions);
> for (p = l->d_partitions; p - l->d_partitions < max_partitions; p++)
> {
> - u32 bsd_start, bsd_size;
> + sector_t bsd_start, bsd_size;
>
> if (state->next == state->limit)
> break;
> if (p->p_fstype == BSD_FS_UNUSED)
> continue;
> - bsd_start = le32_to_cpu(p->p_offset);
> - bsd_size = le32_to_cpu(p->p_size);
> + bsd_start = (sector_t)(le32_to_cpu(p->p_offset));
> + bsd_size = (sector_t)(le32_to_cpu(p->p_size));
I don't see why these two casts are needed? The compiler will take
care of assigning a u32 to a sector_t. Perhaps this applies to other
places in the patch.
> if (offset == bsd_start && size == bsd_size)
> /* full parent partition, we have it already */
> continue;
>
> ...
>
> @@ -483,8 +484,8 @@ int msdos_partition(struct parsed_partitions *state,
> struct block_device *bdev)
>
> state->next = 5;
> for (slot = 1 ; slot <= 4 ; slot++, p++) {
> - u32 start = START_SECT(p)*sector_size;
> - u32 size = NR_SECTS(p)*sector_size;
> + sector_t start =
> (sector_t)(START_SECT(p))*(sector_t)(sector_size);
> + sector_t size =
> (sector_t)(NR_SECTS(p))*(sector_t)(sector_size);
> if (!size)
> continue;
> if (is_extended_partition(p)) {
> @@ -521,8 +522,8 @@ int msdos_partition(struct parsed_partitions *state,
> struct block_device *bdev)
>
> if (!subtypes[n].parse)
> continue;
> - subtypes[n].parse(state, bdev, START_SECT(p)*sector_size,
> - NR_SECTS(p)*sector_size,
> slot);
> + subtypes[n].parse(state, bdev,
> (sector_t)(START_SECT(p))*(sector_t)(sector_size),
> +
> (sector_t)(NR_SECTS(p))*(sector_t)(sector_size), slot);
> }
> put_dev_sector(sect);
> return 1;
Again, we can avoid some/all of this casting by more appropriate
selection of types.
--
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/
Powered by blists - more mailing lists