| Message ID | 20230616223616.6002-3-schmitzmic@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Amiga RDB partition support fixes | expand |
On Sat, Jun 17, 2023 at 12:36 AM Michael Schmitz <schmitzmic@gmail.com> 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. > > 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 patch 1 of this series). > > Patch 3 (this series) adds additional error checking and warning > messages. One of the error checks now makes use of the previously > unused rdb_CylBlocks field, which causes a 'sparse' warning > (cast to restricted __be32). > > Annotate all 32 bit fields in affs_hardblocks.h as __be32, as the > on-disk format of RDB and partition blocks is always big endian. > > Reported-by: Martin Steigerwald <Martin@lichtvoll.de> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=43511 > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Message-ID: <201206192146.09327.Martin@lichtvoll.de> > Cc: <stable@vger.kernel.org> # 5.2 > Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> Gr{oetje,eeting}s, Geert
Hi Geert, Am 17.06.2023 um 23:08 schrieb Geert Uytterhoeven: > On Sat, Jun 17, 2023 at 12:36 AM Michael Schmitz <schmitzmic@gmail.com> 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. >> >> 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 patch 1 of this series). >> >> Patch 3 (this series) adds additional error checking and warning >> messages. One of the error checks now makes use of the previously >> unused rdb_CylBlocks field, which causes a 'sparse' warning >> (cast to restricted __be32). >> >> Annotate all 32 bit fields in affs_hardblocks.h as __be32, as the >> on-disk format of RDB and partition blocks is always big endian. >> >> Reported-by: Martin Steigerwald <Martin@lichtvoll.de> >> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=43511 >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") >> Message-ID: <201206192146.09327.Martin@lichtvoll.de> >> Cc: <stable@vger.kernel.org> # 5.2 >> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> >> Reviewed-by: Christoph Hellwig <hch@lst.de> > > Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> Thanks - now I notice the patch title for this one doesn't fit too well anymore. Would a change of title mess up the common patch tracking tools? Cheers, Michael > > Gr{oetje,eeting}s, > > Geert >
Hi Michael, On Sun, Jun 18, 2023 at 5:10 AM Michael Schmitz <schmitzmic@gmail.com> wrote: > Am 17.06.2023 um 23:08 schrieb Geert Uytterhoeven: > > On Sat, Jun 17, 2023 at 12:36 AM Michael Schmitz <schmitzmic@gmail.com> 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. > >> > >> 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 patch 1 of this series). > >> > >> Patch 3 (this series) adds additional error checking and warning > >> messages. One of the error checks now makes use of the previously > >> unused rdb_CylBlocks field, which causes a 'sparse' warning > >> (cast to restricted __be32). > >> > >> Annotate all 32 bit fields in affs_hardblocks.h as __be32, as the > >> on-disk format of RDB and partition blocks is always big endian. > >> > >> Reported-by: Martin Steigerwald <Martin@lichtvoll.de> > >> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=43511 > >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > >> Message-ID: <201206192146.09327.Martin@lichtvoll.de> > >> Cc: <stable@vger.kernel.org> # 5.2 > >> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> > >> Reviewed-by: Christoph Hellwig <hch@lst.de> > > > > Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> > > Thanks - now I notice the patch title for this one doesn't fit too well > anymore. > > Would a change of title mess up the common patch tracking tools? You mean changing one patch subject in v13? Nah, happens all the time, so the tooling should handle that fine. Gr{oetje,eeting}s, Geert
Hi Geert, Am 18.06.2023 um 19:51 schrieb Geert Uytterhoeven: > Hi Michael, > > On Sun, Jun 18, 2023 at 5:10 AM Michael Schmitz <schmitzmic@gmail.com> wrote: >> Am 17.06.2023 um 23:08 schrieb Geert Uytterhoeven: >>> On Sat, Jun 17, 2023 at 12:36 AM Michael Schmitz <schmitzmic@gmail.com> 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. >>>> >>>> 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 patch 1 of this series). >>>> >>>> Patch 3 (this series) adds additional error checking and warning >>>> messages. One of the error checks now makes use of the previously >>>> unused rdb_CylBlocks field, which causes a 'sparse' warning >>>> (cast to restricted __be32). >>>> >>>> Annotate all 32 bit fields in affs_hardblocks.h as __be32, as the >>>> on-disk format of RDB and partition blocks is always big endian. >>>> >>>> Reported-by: Martin Steigerwald <Martin@lichtvoll.de> >>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=43511 >>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") >>>> Message-ID: <201206192146.09327.Martin@lichtvoll.de> >>>> Cc: <stable@vger.kernel.org> # 5.2 >>>> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> >>>> Reviewed-by: Christoph Hellwig <hch@lst.de> >>> >>> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> >> >> Thanks - now I notice the patch title for this one doesn't fit too well >> anymore. >> >> Would a change of title mess up the common patch tracking tools? > > You mean changing one patch subject in v13? Correct. > Nah, happens all the time, so the tooling should handle that fine. OK - I need to add your review tag anyway. Cheers, Michael > > Gr{oetje,eeting}s, > > Geert >
diff --git a/include/uapi/linux/affs_hardblocks.h b/include/uapi/linux/affs_hardblocks.h index 5e2fb8481252..a5aff2eb5f70 100644 --- a/include/uapi/linux/affs_hardblocks.h +++ b/include/uapi/linux/affs_hardblocks.h @@ -7,42 +7,42 @@ /* Just the needed definitions for the RDB of an Amiga HD. */ struct RigidDiskBlock { - __u32 rdb_ID; + __be32 rdb_ID; __be32 rdb_SummedLongs; - __s32 rdb_ChkSum; - __u32 rdb_HostID; + __be32 rdb_ChkSum; + __be32 rdb_HostID; __be32 rdb_BlockBytes; - __u32 rdb_Flags; - __u32 rdb_BadBlockList; + __be32 rdb_Flags; + __be32 rdb_BadBlockList; __be32 rdb_PartitionList; - __u32 rdb_FileSysHeaderList; - __u32 rdb_DriveInit; - __u32 rdb_Reserved1[6]; - __u32 rdb_Cylinders; - __u32 rdb_Sectors; - __u32 rdb_Heads; - __u32 rdb_Interleave; - __u32 rdb_Park; - __u32 rdb_Reserved2[3]; - __u32 rdb_WritePreComp; - __u32 rdb_ReducedWrite; - __u32 rdb_StepRate; - __u32 rdb_Reserved3[5]; - __u32 rdb_RDBBlocksLo; - __u32 rdb_RDBBlocksHi; - __u32 rdb_LoCylinder; - __u32 rdb_HiCylinder; - __u32 rdb_CylBlocks; - __u32 rdb_AutoParkSeconds; - __u32 rdb_HighRDSKBlock; - __u32 rdb_Reserved4; + __be32 rdb_FileSysHeaderList; + __be32 rdb_DriveInit; + __be32 rdb_Reserved1[6]; + __be32 rdb_Cylinders; + __be32 rdb_Sectors; + __be32 rdb_Heads; + __be32 rdb_Interleave; + __be32 rdb_Park; + __be32 rdb_Reserved2[3]; + __be32 rdb_WritePreComp; + __be32 rdb_ReducedWrite; + __be32 rdb_StepRate; + __be32 rdb_Reserved3[5]; + __be32 rdb_RDBBlocksLo; + __be32 rdb_RDBBlocksHi; + __be32 rdb_LoCylinder; + __be32 rdb_HiCylinder; + __be32 rdb_CylBlocks; + __be32 rdb_AutoParkSeconds; + __be32 rdb_HighRDSKBlock; + __be32 rdb_Reserved4; char rdb_DiskVendor[8]; char rdb_DiskProduct[16]; char rdb_DiskRevision[4]; char rdb_ControllerVendor[8]; char rdb_ControllerProduct[16]; char rdb_ControllerRevision[4]; - __u32 rdb_Reserved5[10]; + __be32 rdb_Reserved5[10]; }; #define IDNAME_RIGIDDISK 0x5244534B /* "RDSK" */ @@ -50,16 +50,16 @@ struct RigidDiskBlock { struct PartitionBlock { __be32 pb_ID; __be32 pb_SummedLongs; - __s32 pb_ChkSum; - __u32 pb_HostID; + __be32 pb_ChkSum; + __be32 pb_HostID; __be32 pb_Next; - __u32 pb_Flags; - __u32 pb_Reserved1[2]; - __u32 pb_DevFlags; + __be32 pb_Flags; + __be32 pb_Reserved1[2]; + __be32 pb_DevFlags; __u8 pb_DriveName[32]; - __u32 pb_Reserved2[15]; + __be32 pb_Reserved2[15]; __be32 pb_Environment[17]; - __u32 pb_EReserved[15]; + __be32 pb_EReserved[15]; }; #define IDNAME_PARTITION 0x50415254 /* "PART" */
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. 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 patch 1 of this series). Patch 3 (this series) adds additional error checking and warning messages. One of the error checks now makes use of the previously unused rdb_CylBlocks field, which causes a 'sparse' warning (cast to restricted __be32). Annotate all 32 bit fields in affs_hardblocks.h as __be32, as the on-disk format of RDB and partition blocks is always big endian. Reported-by: Martin Steigerwald <Martin@lichtvoll.de> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=43511 Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Message-ID: <201206192146.09327.Martin@lichtvoll.de> Cc: <stable@vger.kernel.org> # 5.2 Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> Reviewed-by: Christoph Hellwig <hch@lst.de> --- Changes from v10: Christoph Hellwig: - change annotation of all __u32 fields to __be32 Changes from v11: Geert Uytterhoeven: - also change annotation of the two __s32 checksum fields --- include/uapi/linux/affs_hardblocks.h | 68 ++++++++++++++-------------- 1 file changed, 34 insertions(+), 34 deletions(-)