| Message ID | 20220726045747.4779-3-schmitzmic@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Amiga RDB partition support fixes | expand |
On Tue, Jul 26, 2022 at 04:57:47PM +1200, Michael Schmitz wrote: > The Amiga partition parser module uses signed int for partition sector > address and count, which will overflow for disks larger than 1 TB. > > Use u64 as type for sector address and size to allow using disks up to > 2 TB without LBD support, and disks larger than 2 TB with LBD. The RBD > format allows to specify disk sizes up to 2^128 bytes (though native > OS limitations reduce this somewhat, to max 2^68 bytes), so check for > u64 overflow carefully to protect against overflowing sector_t. > > Bail out if sector addresses overflow 32 bits on kernels without LBD > support. > > This bug was reported originally in 2012, and the fix was created by > the RDB author, Joanne Dow <jdow@earthlink.net>. A patch had been > discussed and reviewed on linux-m68k at that time but never officially > submitted (now resubmitted as separate patch). > This patch adds additional error checking and warning messages. > > Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=43511 > Reported-by: Martin Steigerwald <Martin@lichtvoll.de> > Message-ID: <201206192146.09327.Martin@lichtvoll.de> > Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> > Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> > --- > block/partitions/amiga.c | 111 +++++++++++++++++++++++++++++++-------- > 1 file changed, 89 insertions(+), 22 deletions(-) > > diff --git a/block/partitions/amiga.c b/block/partitions/amiga.c > index f98191545d9a..7356b39cbe10 100644 > --- a/block/partitions/amiga.c > +++ b/block/partitions/amiga.c > @@ -11,10 +11,18 @@ > #define pr_fmt(fmt) fmt > > #include <linux/types.h> > +#include <linux/mm_types.h> > +#include <linux/overflow.h> > #include <linux/affs_hardblocks.h> > > #include "check.h" > > +/* magic offsets in partition DosEnvVec */ > +#define NR_HD 3 > +#define NR_SECT 5 > +#define LO_CYL 9 > +#define HI_CYL 10 The last line has a tab after the #define while the previous three don't. Pick one style and stick to it for the others. > if (!data) { > - pr_err("Dev %s: unable to read RDB block %d\n", > - state->disk->disk_name, blk); > + pr_err("Dev %s: unable to read RDB block %llu\n", > + state->disk->disk_name, (u64) blk); No need for the various printk casts, a sector_t is always an unsigned long long.
Hi Michael,
I love your patch! Perhaps something to improve:
[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on linus/master v5.19-rc8 next-20220725]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Michael-Schmitz/Amiga-RDB-partition-support-fixes/20220726-125830
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: sparc-randconfig-s041-20220724 (https://download.01.org/0day-ci/archive/20220726/202207262047.yRImNV4v-lkp@intel.com/config)
compiler: sparc-linux-gcc (GCC) 12.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/ccbc09d2b458d51ac395c4f8ea7cf703f6e83fdf
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Michael-Schmitz/Amiga-RDB-partition-support-fixes/20220726-125830
git checkout ccbc09d2b458d51ac395c4f8ea7cf703f6e83fdf
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=sparc SHELL=/bin/bash block/partitions/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> block/partitions/amiga.c:132:30: sparse: sparse: cast to restricted __be32
block/partitions/amiga.c:133:25: sparse: sparse: cast to restricted __be32
vim +132 block/partitions/amiga.c
35
36 int amiga_partition(struct parsed_partitions *state)
37 {
38 Sector sect;
39 unsigned char *data;
40 struct RigidDiskBlock *rdb;
41 struct PartitionBlock *pb;
42 u64 start_sect, nr_sects;
43 sector_t blk, end_sect;
44 u32 cylblk; /* rdb_CylBlocks = nr_heads*sect_per_track */
45 u32 nr_hd, nr_sect, lo_cyl, hi_cyl;
46 int part, res = 0;
47 unsigned int blksize = 1; /* Multiplier for disk block size */
48 int slot = 1;
49
50 for (blk = 0; ; blk++, put_dev_sector(sect)) {
51 if (blk == RDB_ALLOCATION_LIMIT)
52 goto rdb_done;
53 data = read_part_sector(state, blk, §);
54 if (!data) {
55 pr_err("Dev %s: unable to read RDB block %llu\n",
56 state->disk->disk_name, (u64) blk);
57 res = -1;
58 goto rdb_done;
59 }
60 if (*(__be32 *)data != cpu_to_be32(IDNAME_RIGIDDISK))
61 continue;
62
63 rdb = (struct RigidDiskBlock *)data;
64 if (checksum_block((__be32 *)data, be32_to_cpu(rdb->rdb_SummedLongs) & 0x7F) == 0)
65 break;
66 /* Try again with 0xdc..0xdf zeroed, Windows might have
67 * trashed it.
68 */
69 *(__be32 *)(data+0xdc) = 0;
70 if (checksum_block((__be32 *)data,
71 be32_to_cpu(rdb->rdb_SummedLongs) & 0x7F)==0) {
72 pr_err("Trashed word at 0xd0 in block %llu ignored in checksum calculation\n",
73 (u64) blk);
74 break;
75 }
76
77 pr_err("Dev %s: RDB in block %llu has bad checksum\n",
78 state->disk->disk_name, (u64) blk);
79 }
80
81 /* blksize is blocks per 512 byte standard block */
82 blksize = be32_to_cpu( rdb->rdb_BlockBytes ) / 512;
83
84 {
85 char tmp[7 + 10 + 1 + 1];
86
87 /* Be more informative */
88 snprintf(tmp, sizeof(tmp), " RDSK (%d)", blksize * 512);
89 strlcat(state->pp_buf, tmp, PAGE_SIZE);
90 }
91 blk = be32_to_cpu(rdb->rdb_PartitionList);
92 put_dev_sector(sect);
93 for (part = 1; blk>0 && part<=16; part++, put_dev_sector(sect)) {
94 /* Read in terms partition table understands */
95 if (check_mul_overflow(blk, (sector_t) blksize, &blk)) {
96 pr_err("Dev %s: overflow calculating partition block %llu! Skipping partitions %u and beyond\n",
97 state->disk->disk_name, (u64) blk, part);
98 break;
99 }
100 data = read_part_sector(state, blk, §);
101 if (!data) {
102 pr_err("Dev %s: unable to read partition block %llu\n",
103 state->disk->disk_name, (u64) blk);
104 res = -1;
105 goto rdb_done;
106 }
107 pb = (struct PartitionBlock *)data;
108 blk = be32_to_cpu(pb->pb_Next);
109 if (pb->pb_ID != cpu_to_be32(IDNAME_PARTITION))
110 continue;
111 if (checksum_block((__be32 *)pb, be32_to_cpu(pb->pb_SummedLongs) & 0x7F) != 0 )
112 continue;
113
114 /* RDB gives us more than enough rope to hang ourselves with,
115 * many times over (2^128 bytes if all fields max out).
116 * Some careful checks are in order, so check for potential
117 * overflows.
118 * We are multiplying four 32 bit numbers to one sector_t!
119 */
120
121 nr_hd = be32_to_cpu(pb->pb_Environment[NR_HD]);
122 nr_sect = be32_to_cpu(pb->pb_Environment[NR_SECT]);
123
124 /* CylBlocks is total number of blocks per cylinder */
125 if (check_mul_overflow(nr_hd, nr_sect, &cylblk)) {
126 pr_err("Dev %s: heads*sects %u overflows u32, skipping partition!\n",
127 state->disk->disk_name, cylblk);
128 continue;
129 }
130
131 /* check for consistency with RDB defined CylBlocks */
> 132 if (cylblk > be32_to_cpu(rdb->rdb_CylBlocks)) {
Hi Christoph, On 26/07/22 23:42, Christoph Hellwig wrote: > On Tue, Jul 26, 2022 at 04:57:47PM +1200, Michael Schmitz wrote: >> The Amiga partition parser module uses signed int for partition sector >> address and count, which will overflow for disks larger than 1 TB. >> >> Use u64 as type for sector address and size to allow using disks up to >> 2 TB without LBD support, and disks larger than 2 TB with LBD. The RBD >> format allows to specify disk sizes up to 2^128 bytes (though native >> OS limitations reduce this somewhat, to max 2^68 bytes), so check for >> u64 overflow carefully to protect against overflowing sector_t. >> >> Bail out if sector addresses overflow 32 bits on kernels without LBD >> support. >> >> This bug was reported originally in 2012, and the fix was created by >> the RDB author, Joanne Dow <jdow@earthlink.net>. A patch had been >> discussed and reviewed on linux-m68k at that time but never officially >> submitted (now resubmitted as separate patch). >> This patch adds additional error checking and warning messages. >> >> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=43511 >> Reported-by: Martin Steigerwald <Martin@lichtvoll.de> >> Message-ID: <201206192146.09327.Martin@lichtvoll.de> >> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> >> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> >> --- >> block/partitions/amiga.c | 111 +++++++++++++++++++++++++++++++-------- >> 1 file changed, 89 insertions(+), 22 deletions(-) >> >> diff --git a/block/partitions/amiga.c b/block/partitions/amiga.c >> index f98191545d9a..7356b39cbe10 100644 >> --- a/block/partitions/amiga.c >> +++ b/block/partitions/amiga.c >> @@ -11,10 +11,18 @@ >> #define pr_fmt(fmt) fmt >> >> #include <linux/types.h> >> +#include <linux/mm_types.h> >> +#include <linux/overflow.h> >> #include <linux/affs_hardblocks.h> >> >> #include "check.h" >> >> +/* magic offsets in partition DosEnvVec */ >> +#define NR_HD 3 >> +#define NR_SECT 5 >> +#define LO_CYL 9 >> +#define HI_CYL 10 > The last line has a tab after the #define while the previous three > don't. Pick one style and stick to it for the others. > >> if (!data) { >> - pr_err("Dev %s: unable to read RDB block %d\n", >> - state->disk->disk_name, blk); >> + pr_err("Dev %s: unable to read RDB block %llu\n", >> + state->disk->disk_name, (u64) blk); > No need for the various printk casts, a sector_t is always an > unsigned long long. Thanks, wil fix (and address the other issues raised). Cheers, Michael
Hi Christoph, On Tue, Jul 26, 2022 at 1:43 PM Christoph Hellwig <hch@infradead.org> wrote: > On Tue, Jul 26, 2022 at 04:57:47PM +1200, Michael Schmitz wrote: > > The Amiga partition parser module uses signed int for partition sector > > address and count, which will overflow for disks larger than 1 TB. > > > > Use u64 as type for sector address and size to allow using disks up to > > 2 TB without LBD support, and disks larger than 2 TB with LBD. The RBD > > format allows to specify disk sizes up to 2^128 bytes (though native > > OS limitations reduce this somewhat, to max 2^68 bytes), so check for > > u64 overflow carefully to protect against overflowing sector_t. > > > > Bail out if sector addresses overflow 32 bits on kernels without LBD > > support. > > > > This bug was reported originally in 2012, and the fix was created by > > the RDB author, Joanne Dow <jdow@earthlink.net>. A patch had been > > discussed and reviewed on linux-m68k at that time but never officially > > submitted (now resubmitted as separate patch). > > This patch adds additional error checking and warning messages. > > > > Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=43511 > > Reported-by: Martin Steigerwald <Martin@lichtvoll.de> > > Message-ID: <201206192146.09327.Martin@lichtvoll.de> > > Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> > > Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> > > --- a/block/partitions/amiga.c > > +++ b/block/partitions/amiga.c > > if (!data) { > > - pr_err("Dev %s: unable to read RDB block %d\n", > > - state->disk->disk_name, blk); > > + pr_err("Dev %s: unable to read RDB block %llu\n", > > + state->disk->disk_name, (u64) blk); > > No need for the various printk casts, a sector_t is always an > unsigned long long. That is true, as of commit 72deb455b5ec619f ("block: remove CONFIG_LBDAF") in v5.2. Since 4.9, 4.14, and 4.19 are still receiving stable updates, the cast should be re-added when this is backported. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, On 11/08/22 23:40, Geert Uytterhoeven wrote: > Hi Christoph, > > On Tue, Jul 26, 2022 at 1:43 PM Christoph Hellwig <hch@infradead.org> wrote: >> On Tue, Jul 26, 2022 at 04:57:47PM +1200, Michael Schmitz wrote: >>> The Amiga partition parser module uses signed int for partition sector >>> address and count, which will overflow for disks larger than 1 TB. >>> >>> Use u64 as type for sector address and size to allow using disks up to >>> 2 TB without LBD support, and disks larger than 2 TB with LBD. The RBD >>> format allows to specify disk sizes up to 2^128 bytes (though native >>> OS limitations reduce this somewhat, to max 2^68 bytes), so check for >>> u64 overflow carefully to protect against overflowing sector_t. >>> >>> Bail out if sector addresses overflow 32 bits on kernels without LBD >>> support. >>> >>> This bug was reported originally in 2012, and the fix was created by >>> the RDB author, Joanne Dow <jdow@earthlink.net>. A patch had been >>> discussed and reviewed on linux-m68k at that time but never officially >>> submitted (now resubmitted as separate patch). >>> This patch adds additional error checking and warning messages. >>> >>> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=43511 >>> Reported-by: Martin Steigerwald <Martin@lichtvoll.de> >>> Message-ID: <201206192146.09327.Martin@lichtvoll.de> >>> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> >>> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> >>> --- a/block/partitions/amiga.c >>> +++ b/block/partitions/amiga.c >>> if (!data) { >>> - pr_err("Dev %s: unable to read RDB block %d\n", >>> - state->disk->disk_name, blk); >>> + pr_err("Dev %s: unable to read RDB block %llu\n", >>> + state->disk->disk_name, (u64) blk); >> No need for the various printk casts, a sector_t is always an >> unsigned long long. > That is true, as of commit 72deb455b5ec619f > ("block: remove CONFIG_LBDAF") in v5.2. > Since 4.9, 4.14, and 4.19 are still receiving stable updates, the > cast should be re-added when this is backported. Does this require a note in the commit message, or explicit CC to Greg? Cheers, Michael > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
Hi Michael, On Mon, Aug 22, 2022 at 10:38 PM Michael Schmitz <schmitzmic@gmail.com> wrote: > On 11/08/22 23:40, Geert Uytterhoeven wrote: > > On Tue, Jul 26, 2022 at 1:43 PM Christoph Hellwig <hch@infradead.org> wrote: > >> On Tue, Jul 26, 2022 at 04:57:47PM +1200, Michael Schmitz wrote: > >>> The Amiga partition parser module uses signed int for partition sector > >>> address and count, which will overflow for disks larger than 1 TB. > >>> > >>> Use u64 as type for sector address and size to allow using disks up to > >>> 2 TB without LBD support, and disks larger than 2 TB with LBD. The RBD > >>> format allows to specify disk sizes up to 2^128 bytes (though native > >>> OS limitations reduce this somewhat, to max 2^68 bytes), so check for > >>> u64 overflow carefully to protect against overflowing sector_t. > >>> > >>> Bail out if sector addresses overflow 32 bits on kernels without LBD > >>> support. > >>> > >>> This bug was reported originally in 2012, and the fix was created by > >>> the RDB author, Joanne Dow <jdow@earthlink.net>. A patch had been > >>> discussed and reviewed on linux-m68k at that time but never officially > >>> submitted (now resubmitted as separate patch). > >>> This patch adds additional error checking and warning messages. > >>> > >>> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=43511 > >>> Reported-by: Martin Steigerwald <Martin@lichtvoll.de> > >>> Message-ID: <201206192146.09327.Martin@lichtvoll.de> > >>> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> > >>> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> > >>> --- a/block/partitions/amiga.c > >>> +++ b/block/partitions/amiga.c > >>> if (!data) { > >>> - pr_err("Dev %s: unable to read RDB block %d\n", > >>> - state->disk->disk_name, blk); > >>> + pr_err("Dev %s: unable to read RDB block %llu\n", > >>> + state->disk->disk_name, (u64) blk); > >> No need for the various printk casts, a sector_t is always an > >> unsigned long long. > > That is true, as of commit 72deb455b5ec619f > > ("block: remove CONFIG_LBDAF") in v5.2. > > Since 4.9, 4.14, and 4.19 are still receiving stable updates, the > > cast should be re-added when this is backported. > > Does this require a note in the commit message, or explicit CC to Greg? According to [1], you should add Cc: <stable@vger.kernel.org> # 5.2 [1] https://docs.kernel.org/process/stable-kernel-rules.html?highlight=prerequisites Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, saw this a minute too late - will resend again. Cheers, Michael On 23/08/22 09:03, Geert Uytterhoeven wrote: > Hi Michael, > > On Mon, Aug 22, 2022 at 10:38 PM Michael Schmitz <schmitzmic@gmail.com> wrote: >> On 11/08/22 23:40, Geert Uytterhoeven wrote: >>> On Tue, Jul 26, 2022 at 1:43 PM Christoph Hellwig <hch@infradead.org> wrote: >>>> On Tue, Jul 26, 2022 at 04:57:47PM +1200, Michael Schmitz wrote: >>>>> The Amiga partition parser module uses signed int for partition sector >>>>> address and count, which will overflow for disks larger than 1 TB. >>>>> >>>>> Use u64 as type for sector address and size to allow using disks up to >>>>> 2 TB without LBD support, and disks larger than 2 TB with LBD. The RBD >>>>> format allows to specify disk sizes up to 2^128 bytes (though native >>>>> OS limitations reduce this somewhat, to max 2^68 bytes), so check for >>>>> u64 overflow carefully to protect against overflowing sector_t. >>>>> >>>>> Bail out if sector addresses overflow 32 bits on kernels without LBD >>>>> support. >>>>> >>>>> This bug was reported originally in 2012, and the fix was created by >>>>> the RDB author, Joanne Dow <jdow@earthlink.net>. A patch had been >>>>> discussed and reviewed on linux-m68k at that time but never officially >>>>> submitted (now resubmitted as separate patch). >>>>> This patch adds additional error checking and warning messages. >>>>> >>>>> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=43511 >>>>> Reported-by: Martin Steigerwald <Martin@lichtvoll.de> >>>>> Message-ID: <201206192146.09327.Martin@lichtvoll.de> >>>>> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> >>>>> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> >>>>> --- a/block/partitions/amiga.c >>>>> +++ b/block/partitions/amiga.c >>>>> if (!data) { >>>>> - pr_err("Dev %s: unable to read RDB block %d\n", >>>>> - state->disk->disk_name, blk); >>>>> + pr_err("Dev %s: unable to read RDB block %llu\n", >>>>> + state->disk->disk_name, (u64) blk); >>>> No need for the various printk casts, a sector_t is always an >>>> unsigned long long. >>> That is true, as of commit 72deb455b5ec619f >>> ("block: remove CONFIG_LBDAF") in v5.2. >>> Since 4.9, 4.14, and 4.19 are still receiving stable updates, the >>> cast should be re-added when this is backported. >> Does this require a note in the commit message, or explicit CC to Greg? > According to [1], you should add > > Cc: <stable@vger.kernel.org> # 5.2 > > [1] https://docs.kernel.org/process/stable-kernel-rules.html?highlight=prerequisites > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
diff --git a/block/partitions/amiga.c b/block/partitions/amiga.c index f98191545d9a..7356b39cbe10 100644 --- a/block/partitions/amiga.c +++ b/block/partitions/amiga.c @@ -11,10 +11,18 @@ #define pr_fmt(fmt) fmt #include <linux/types.h> +#include <linux/mm_types.h> +#include <linux/overflow.h> #include <linux/affs_hardblocks.h> #include "check.h" +/* magic offsets in partition DosEnvVec */ +#define NR_HD 3 +#define NR_SECT 5 +#define LO_CYL 9 +#define HI_CYL 10 + static __inline__ u32 checksum_block(__be32 *m, int size) { @@ -31,9 +39,12 @@ int amiga_partition(struct parsed_partitions *state) unsigned char *data; struct RigidDiskBlock *rdb; struct PartitionBlock *pb; - sector_t start_sect, nr_sects; - int blk, part, res = 0; - int blksize = 1; /* Multiplier for disk block size */ + u64 start_sect, nr_sects; + sector_t blk, end_sect; + u32 cylblk; /* rdb_CylBlocks = nr_heads*sect_per_track */ + u32 nr_hd, nr_sect, lo_cyl, hi_cyl; + int part, res = 0; + unsigned int blksize = 1; /* Multiplier for disk block size */ int slot = 1; for (blk = 0; ; blk++, put_dev_sector(sect)) { @@ -41,8 +52,8 @@ int amiga_partition(struct parsed_partitions *state) goto rdb_done; data = read_part_sector(state, blk, §); if (!data) { - pr_err("Dev %s: unable to read RDB block %d\n", - state->disk->disk_name, blk); + pr_err("Dev %s: unable to read RDB block %llu\n", + state->disk->disk_name, (u64) blk); res = -1; goto rdb_done; } @@ -58,13 +69,13 @@ int amiga_partition(struct parsed_partitions *state) *(__be32 *)(data+0xdc) = 0; if (checksum_block((__be32 *)data, be32_to_cpu(rdb->rdb_SummedLongs) & 0x7F)==0) { - pr_err("Trashed word at 0xd0 in block %d ignored in checksum calculation\n", - blk); + pr_err("Trashed word at 0xd0 in block %llu ignored in checksum calculation\n", + (u64) blk); break; } - pr_err("Dev %s: RDB in block %d has bad checksum\n", - state->disk->disk_name, blk); + pr_err("Dev %s: RDB in block %llu has bad checksum\n", + state->disk->disk_name, (u64) blk); } /* blksize is blocks per 512 byte standard block */ @@ -80,11 +91,16 @@ int amiga_partition(struct parsed_partitions *state) blk = be32_to_cpu(rdb->rdb_PartitionList); put_dev_sector(sect); for (part = 1; blk>0 && part<=16; part++, put_dev_sector(sect)) { - blk *= blksize; /* Read in terms partition table understands */ + /* Read in terms partition table understands */ + if (check_mul_overflow(blk, (sector_t) blksize, &blk)) { + pr_err("Dev %s: overflow calculating partition block %llu! Skipping partitions %u and beyond\n", + state->disk->disk_name, (u64) blk, part); + break; + } data = read_part_sector(state, blk, §); if (!data) { - pr_err("Dev %s: unable to read partition block %d\n", - state->disk->disk_name, blk); + pr_err("Dev %s: unable to read partition block %llu\n", + state->disk->disk_name, (u64) blk); res = -1; goto rdb_done; } @@ -95,19 +111,70 @@ int amiga_partition(struct parsed_partitions *state) if (checksum_block((__be32 *)pb, be32_to_cpu(pb->pb_SummedLongs) & 0x7F) != 0 ) continue; - /* Tell Kernel about it */ + /* RDB gives us more than enough rope to hang ourselves with, + * many times over (2^128 bytes if all fields max out). + * Some careful checks are in order, so check for potential + * overflows. + * We are multiplying four 32 bit numbers to one sector_t! + */ + + nr_hd = be32_to_cpu(pb->pb_Environment[NR_HD]); + nr_sect = be32_to_cpu(pb->pb_Environment[NR_SECT]); + + /* CylBlocks is total number of blocks per cylinder */ + if (check_mul_overflow(nr_hd, nr_sect, &cylblk)) { + pr_err("Dev %s: heads*sects %u overflows u32, skipping partition!\n", + state->disk->disk_name, cylblk); + continue; + } + + /* check for consistency with RDB defined CylBlocks */ + if (cylblk > be32_to_cpu(rdb->rdb_CylBlocks)) { + pr_warn("Dev %s: cylblk %u > rdb_CylBlocks %u!\n", + state->disk->disk_name, cylblk, + be32_to_cpu(rdb->rdb_CylBlocks)); + } + + /* RDB allows for variable logical block size - + * normalize to 512 byte blocks and check result. + */ + + if (check_mul_overflow(cylblk, blksize, &cylblk)) { + pr_err("Dev %s: partition %u bytes per cyl. overflows u32, skipping partition!\n", + state->disk->disk_name, part); + continue; + } + + /* Calculate partition start and end. Limit of 32 bit on cylblk + * guarantees no overflow occurs if LBD support is enabled. + */ + + lo_cyl = be32_to_cpu(pb->pb_Environment[LO_CYL]); + start_sect = ((u64) lo_cyl * cylblk); + + hi_cyl = be32_to_cpu(pb->pb_Environment[HI_CYL]); + nr_sects = (((u64) hi_cyl - lo_cyl + 1) * cylblk); - nr_sects = ((sector_t) be32_to_cpu(pb->pb_Environment[10]) - + 1 - be32_to_cpu(pb->pb_Environment[9])) * - be32_to_cpu(pb->pb_Environment[3]) * - be32_to_cpu(pb->pb_Environment[5]) * - blksize; if (!nr_sects) continue; - start_sect = (sector_t) be32_to_cpu(pb->pb_Environment[9]) * - be32_to_cpu(pb->pb_Environment[3]) * - be32_to_cpu(pb->pb_Environment[5]) * - blksize; + + /* Warn user if partition end overflows u32 (AmigaDOS limit) */ + + if ((start_sect + nr_sects) > UINT_MAX) { + pr_warn("Dev %s: partition %u (%llu-%llu) needs 64 bit device support!\n", + state->disk->disk_name, part, + start_sect, start_sect + nr_sects); + } + + if (check_add_overflow(start_sect, nr_sects, &end_sect)) { + pr_err("Dev %s: partition %u (%llu-%llu) needs LBD device support, skipping partition!\n", + state->disk->disk_name, part, + start_sect, (u64) end_sect); + continue; + } + + /* Tell Kernel about it */ + put_partition(state,slot++,start_sect,nr_sects); { /* Be even more informative to aid mounting */