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 for Android: free password hash cracker in your pocket
[<prev] [next>] [day] [month] [year] [list]
Date:   Wed, 19 Oct 2022 20:05:38 -0700
From:   Kees Cook <>
To:     "Darrick J. Wong" <>
Cc:     xfs <>, Zorro Lang <>,
Subject: Re: [RFC PATCH] xfs: fix FORTIFY_SOURCE complaints about log item

On Wed, Oct 19, 2022 at 05:04:11PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <>
> Starting in 6.1, CONFIG_FORTIFY_SOURCE checks the length parameter of
> memcpy.  Unfortunately, it doesn't handle VLAs correctly:

Nit-pick on terminology: these are "flexible array structures" (structures
that end with a "flexible array member"); VLAs are a different (removed
from the kernel) beast.

> memcpy: detected field-spanning write (size 48) of single field "dst_bui_fmt" at fs/xfs/xfs_bmap_item.c:628 (size 16)

Step right up; XFS is next to trip[1] this check. Let's get this fixed...

> We know the memcpy going
> on here is correct because I've run all the log recovery tests with
> KASAN turned on, and it does not detect actual memory misuse.

Yup, this is a false positive.

> My first attempt to work around this problem was to cast the arguments
> [...]
> My second attempt changed the cast to a (void *), with the same results
> [...]
> My third attempt was to pass the void pointers directly into
> [...]
> My fourth attempt collapsed the _copy_format function into the callers
> [...]

The point here is to use a better API, which is fallible and has the
ability to perform the bounds checking itself. I had proposed an initial
version of this idea here[2].


> "These cases end up appearing to the compiler to be sized as if the
> flexible array had 0 elements. :( For more details see:
> ".
> I don't /quite/ think that turning off CONFIG_FORTIFY_SOURCE is the
> right solution here, but in the meantime this is causing a lot of fstest
> failures, and I really need to get back to fixing user reported data
> corruption problems instead of dealing with gcc stupidity. :(

I think XFS could be a great first candidate for using something close
to the proposed flex_cpy() API. What do you think of replacing the
memcpy() calls with something like this instead:

-	if (buf->i_len == len) {
-		memcpy(dst_bui_fmt, src_bui_fmt, len);
-		return 0;
-	}
+	if (buf->i_len == len &&
+	    flex_cpy(dst_bui_fmt, src_bui_fmt,
+		     bui_nextents, bui_extents) == 0)
		return 0;

To avoid passing in the element count and element array fields, the
alias macros could be used:

struct xfs_bui_log_format {
	uint16_t		bui_type;	/* bui log item type */
	uint16_t		bui_size;	/* size of this item */
	/* # extents to free */
	DECLARE_FLEX_ARRAY_ELEMENTS_COUNT(uint32_t, bui_nextents);
	uint64_t		bui_id;		/* bui identifier */
	/* array of extents to bmap */
	DECLARE_FLEX_ARRAY_ELEMENTS(struct xfs_map_extent, bui_extents);

What do you think about these options? In the meantime, unsafe_memcpy()
should be fine for v6.1.

BTW, this FORTIFY_SOURCE change was present in linux-next for the entire
prior development cycle. Are the xfstests not run on -next kernels?


Kees Cook

Powered by blists - more mailing lists