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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 30 Apr 2020 23:54:50 +0200
From:   Pali Rohár <pali@...nel.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     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 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.

> 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.

Well, this code uses GNU extension for zero-length arrays because it was
written in pre-C99 era when C99 flexible arrays did not exist yet.

> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
>  fs/udf/ecma_167.h | 2 +-
>  fs/udf/super.c    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/udf/ecma_167.h b/fs/udf/ecma_167.h
> index 736ebc5dc441..14ffe27342bc 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];
> +	__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.

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.

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

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.

>  } __packed;
>  
>  /* Integrity Type (ECMA 167r3 3/10.10.3) */
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index f747bf72edbe..379867888c36 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -117,7 +117,7 @@ struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu(struct super_block *sb)
>  	}
>  	/* The offset is to skip freeSpaceTable and sizeTable arrays */
>  	offset = partnum * 2 * sizeof(uint32_t);
> -	return (struct logicalVolIntegrityDescImpUse *)&(lvid->impUse[offset]);
> +	return (struct logicalVolIntegrityDescImpUse *)(lvid->impUse + offset);
>  }
>  
>  /* UDF filesystem type */
> -- 
> 2.26.0
> 

Powered by blists - more mailing lists