linux-kernel - Re: [PATCH 09/15] udf: avoid gcc-10 zero-length-bounds warnings

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200430215450.anfwm4zikvhy2bt5@pali>
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
>