| 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
| ||
|
Message-ID: <CAK8P3a0_BuKDK7X8dTHyx7t++CGjy3jhDVvL=Jo5=kcUxmtobg@mail.gmail.com>
Date: Fri, 1 May 2020 22:30:27 +0200
From: Arnd Bergmann <arnd@...db.de>
To: Pali Rohár <pali@...nel.org>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Jan Kara <jack@...e.com>,
Andrew Morton <akpm@...ux-foundation.org>,
"Steven J. Magnani" <steve@...idescorp.com>,
Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH 09/15] udf: avoid gcc-10 zero-length-bounds warnings
On Thu, Apr 30, 2020 at 11:54 PM Pali Rohár <pali@...nel.org> wrote:
>
> On Thursday 30 April 2020 23:30:51 Arnd Bergmann wrote:
> > gcc-10 warns about writes to the empty freeSpaceTable[] array, with
> > many instances like:
> >
> > fs/udf/balloc.c: In function 'udf_bitmap_new_block':
> > fs/udf/balloc.c:101:36: error: array subscript 65535 is outside the bounds of an interior zero-length array '__le32[0]' {aka 'unsigned int[0]'} [-Werror=zero-length-bounds]
> > 101 | le32_add_cpu(&lvid->freeSpaceTable[partition], cnt);
> > | ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
> > In file included from fs/udf/udfdecl.h:7,
> > from fs/udf/balloc.c:22:
> > fs/udf/ecma_167.h:363:11: note: while referencing 'freeSpaceTable'
> > 363 | __le32 freeSpaceTable[0];
> > | ^~~~~~~~~~~~~~
>
> Hi Arnd! This looks like a false-positive warning.
Right, sorry for not making that clearer in the changelog.
> > These can all be avoided by using a flexible array member instead.
> >
> > Another warning is a bit more obscure:
> >
> > fs/udf/super.c: In function 'udf_count_free':
> > fs/udf/super.c:2521:26: warning: array subscript '(<unknown>) + 4294967295' is outside the bounds of an interior zero-length array '__le32[0]' {aka 'unsigned int[0]'} [-Wzero-length-bounds]
> > 2521 | lvid->freeSpaceTable[part]);
> >
> > Work around this one by changing the array access to equivalent
> > pointer arithmetic, as there cannot be multiple flexible-array
> > members in a single struct.
>
> > @@ -360,9 +360,9 @@ struct logicalVolIntegrityDesc {
> > uint8_t logicalVolContentsUse[32];
> > __le32 numOfPartitions;
> > __le32 lengthOfImpUse;
> > - __le32 freeSpaceTable[0];
> > __le32 sizeTable[0];
> > uint8_t impUse[0];
> > + __le32 freeSpaceTable[];
>
> Please do not change order of members in these structures. Order is
> strictly defined by ECMA 167 standard and changing them you would just
> confuse reader. In LVID is free space table before size table.
Ok
> If you do not like GNU C extension for zero-length arrays then just
> replace it by standard C99 flexible arrays. I think that there is no
> reason to not use standard C99 language constructions, just nobody had
> motivation or time to change (working) code.
No, the problem is that only the last member can be a flexible array,
so when impUse[] is the last member, freeSpaceTable has to be a zero
length array.
[]> Also this file is semi-synchronized with udftools project in which I
> already replaced all GNU C zero-length arrays by C99 flexible arrays.
>
> You can take inspiration what I did with logicalVolIntegrityDesc:
> https://github.com/pali/udftools/commit/f851d84478ce881d516a76018745fa163f803880#diff-1e1a5b89f620d380f22b973f9449aeaeL381-R384
Right, this is likely the best workaround.
> Anyway, if you have a better idea what to do with such on-disk structure
> and how to represent it in C struct syntax, let me know as it could be
> updated also in udftools project.
The trick I used for impUse[] would also work for freeSpaceTable[] to avoid
the gcc warning, it's still not great, but maybe you like this better:
arnd@...eadripper:~/arm-soc$ git diff
diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
index 02f03fadb75b..666d022eb00b 100644
--- a/fs/udf/balloc.c
+++ b/fs/udf/balloc.c
@@ -98,7 +98,7 @@ static void udf_add_free_space(struct super_block
*sb, u16 partition, u32 cnt)
return;
lvid = (struct logicalVolIntegrityDesc *)sbi->s_lvid_bh->b_data;
- le32_add_cpu(&lvid->freeSpaceTable[partition], cnt);
+ le32_add_cpu(lvid->freeSpaceTable + partition, cnt);
udf_updated_lvid(sb);
}
diff --git a/fs/udf/ecma_167.h b/fs/udf/ecma_167.h
index 14ffe27342bc..215d97d7edc4 100644
--- a/fs/udf/ecma_167.h
+++ b/fs/udf/ecma_167.h
@@ -360,9 +360,9 @@ struct logicalVolIntegrityDesc {
uint8_t logicalVolContentsUse[32];
__le32 numOfPartitions;
__le32 lengthOfImpUse;
__le32 freeSpaceTable[0];
__le32 sizeTable[0];
- uint8_t impUse[0];
+ uint8_t impUse[];
} __packed;
/* Integrity Type (ECMA 167r3 3/10.10.3) */
diff --git a/fs/udf/super.c b/fs/udf/super.c
index 379867888c36..a1fc51c2261e 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -2517,8 +2517,8 @@ static unsigned int udf_count_free(struct super_block *sb)
(struct logicalVolIntegrityDesc *)
sbi->s_lvid_bh->b_data;
if (le32_to_cpu(lvid->numOfPartitions) > part) {
- accum = le32_to_cpu(
- lvid->freeSpaceTable[part]);
+ accum = le32_to_cpup(
+ (lvid->freeSpaceTable + part));
if (accum == 0xFFFFFFFF)
accum = 0;
}
This version could easily be backported to stable kernels to let them be
compiled with gcc-10, and then synchronizing with the udftools version of
the header needs additional changes on top, which do not need to be
backported.
Arnd
Powered by blists - more mailing lists