Resolve various build issues #13575

behlendorf · 2022-06-21T00:18:46Z

Motivation and Context

See issue #13528 and Fedora 35 CI failures.

Resolve build warnings observed on Fedora 36 with:

FORTIFY_SOURCE
5.19.0-0.rc0.20220526gitbabf0bb978e3.4.vanilla.1.fc36.x86_64 kernel
gcc 12.1.1

Description

See commit messages for descriptions.

78215e8 Fix objtool: missing int3 after ret warning
60be8c3 Fix -Wformat-overflow warning in zfs_project_handle_dir()
9c0f698 Fix -Wformat-truncation warning in upgrade_set_callback()
02bfbcd Fix -Wuse-after-free warning in dbuf_destroy()
ada1d35 Fix -Wuse-after-free warning in dbuf_issue_final_prefetch_done()
9a5c3c0 Fix -Wattribute-warning in dsl layer
e3a410d Fix -Wattribute-warning in edonr
eadde04 Silence -Wattribute-warning in zfs_log_xvattr()
936a4e4 Silence -Winfinite-recursion warning in luaD_throw()

How Has This Been Tested?

Compiled on Fedora 36 and RHEL 8.6.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Performance enhancement (non-breaking change which improves efficiency)
Code cleanup (non-breaking change which makes code smaller or more readable)
Breaking change (fix or feature that would cause existing functionality to change)
Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the OpenZFS code style requirements.
I have updated the documentation accordingly.
I have read the contributing document.
I have added tests to cover my changes.
I have run the ZFS Test Suite with this change applied.
All commit messages are properly formatted and contain Signed-off-by.

behlendorf · 2022-06-24T20:25:02Z

@AttilaFueloep would you mind taking a look at this commit in the PR which resolves a new objtool warning. As I understand it this is the recommended mitigation.

78215e8 Fix objtool: missing int3 after ret warning

[edit] @freqlabs I wasn't sure how FreeBSD would want to handle this so I opted to only add the int3 arg on Linux. Please let me know if I should conditionally enable this mitigation for FreeBSD as well (and how).

cmd/zfs/zfs_project.c

This code should be kept inline with the upstream lua version as much as possible. Therefore, we simply want to silence the warning. This check was enabled by default as part of -Wall in gcc 12.1. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>

Restructure the code in zfs_log_xvattr() to use a lr_attr_end structure when accessing lr_attr_t elements located after the variable sized array. This makes the code more understandable and resolves the accessing beyond the end of the field warnings. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>

The wrong union memory was being accessed in EdonRInit resulting in a write beyond size of field compiler warning. Reference the correct member to resolve the warning. The warning was correct and this in case the mistake was harmless. In function ‘fortify_memcpy_chk’, inlined from ‘EdonRInit’ at zfs/module/icp/algs/edonr/edonr.c:494:3: ./include/linux/fortify-string.h:344:25: error: call to ‘__write_overflow_field’ declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning] Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>

The memcpy(), memmove(), and memset() functions have been annotated to perform bounds checking when using FORTIFY_SOURCE. A warning is now generted when writing beyond the end of the specified field. Alternately, the new struct_group() macro could be used to create an anonymous union member for use by memcpy(). However, since this is the only place the macro would be helpful it's preferable to restructure the code slights to avoid the need for additional compatibility code when the macro does not exist. https://lore.kernel.org/lkml/20211118183807.1283332-1-keescook@chromium.org/T/ Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>

Move the use of the private pointer after it is freed. It's only used as a tag so a dereference would never occur, but there's no harm in inverting the order to resolve the warning. module/zfs/dbuf.c: In function 'dbuf_issue_final_prefetch_done': module/zfs/dbuf.c:3204:17: error: pointer 'private' may be used after 'free' [-Werror=use-after-free] Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>

Move the use of the db pointer after it is freed. It's only used as a tag so a dereference would never occur, but there's no reason we can't invert the order to resolve the warning. module/zfs/dbuf.c: In function 'dbuf_destroy': module/zfs/dbuf.c:2953:17: error: pointer 'db' may be used after 'free' [-Werror=use-after-free] Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>

Extend the buffer slightly resolve the warning. cmd/zfs/zfs_main.c: In function ‘upgrade_set_callback’: cmd/zfs/zfs_main.c:2446:22: error: ‘%llu’ directive output may be truncated writing between 1 and 20 bytes into a region of size 16 [-Werror=format-truncation=] cmd/zfs/zfs_main.c:2445:24: note: ‘snprintf’ output between 2 and 21 bytes into a destination of size 16 Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>

ghost · 2022-06-24T22:05:36Z

[edit] @freqlabs I wasn't sure how FreeBSD would want to handle this so I opted to only add the int3 arg on Linux. Please let me know if I should conditionally enable this mitigation for FreeBSD as well (and how).

I don't think FreeBSD does straight-line speculation mitigations anywhere else, so this should be fine. @markjdb or @kostikbel may chime in if I'm mistaken.

amotin

I've found only one minor issue, and it is not yours.

cmd/zfs/zfs_project.c

Switch to using asprintf() to satisfy the compiler and resolve the potential format-overflow warning. Not the conditional before the sprintf() would have prevented this regardless. cmd/zfs/zfs_project.c: In function ‘zfs_project_handle_dir’: cmd/zfs/zfs_project.c:241:38: error: ‘/’ directive writing 1 byte into a region of size between 0 and 4352 [-Werror=format-overflow=] cmd/zfs/zfs_project.c:241:17: note: ‘sprintf’ output between 2 and 4609 bytes into a destination of size 4352 Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>

Resolve straight-line speculation warnings reported by objtool for x86_64 assembly on Linux when CONFIG_SLS is set. See the following LWN article for the complete details. https://lwn.net/Articles/877845/ Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>

markjdb · 2022-06-27T18:39:52Z

I don't think FreeBSD does straight-line speculation mitigations anywhere else, so this should be fine. @markjdb or @kostikbel may chime in if I'm mistaken.

Indeed, we don't have mitigations in place for this one.

The wrong union memory was being accessed in EdonRInit resulting in a write beyond size of field compiler warning. Reference the correct member to resolve the warning. The warning was correct and this in case the mistake was harmless. In function ‘fortify_memcpy_chk’, inlined from ‘EdonRInit’ at zfs/module/icp/algs/edonr/edonr.c:494:3: ./include/linux/fortify-string.h:344:25: error: call to ‘__write_overflow_field’ declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning] Reviewed-by: Ryan Moeller <ryan@iXsystems.com> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#13528 Closes openzfs#13575

The memcpy(), memmove(), and memset() functions have been annotated to perform bounds checking when using FORTIFY_SOURCE. A warning is now generted when writing beyond the end of the specified field. Alternately, the new struct_group() macro could be used to create an anonymous union member for use by memcpy(). However, since this is the only place the macro would be helpful it's preferable to restructure the code slights to avoid the need for additional compatibility code when the macro does not exist. https://lore.kernel.org/lkml/20211118183807.1283332-1-keescook@chromium.org/T/ Reviewed-by: Ryan Moeller <ryan@iXsystems.com> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#13528 Closes openzfs#13575

Move the use of the private pointer after it is freed. It's only used as a tag so a dereference would never occur, but there's no harm in inverting the order to resolve the warning. module/zfs/dbuf.c: In function 'dbuf_issue_final_prefetch_done': module/zfs/dbuf.c:3204:17: error: pointer 'private' may be used after 'free' [-Werror=use-after-free] Reviewed-by: Ryan Moeller <ryan@iXsystems.com> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#13528 Closes openzfs#13575

Move the use of the db pointer after it is freed. It's only used as a tag so a dereference would never occur, but there's no reason we can't invert the order to resolve the warning. module/zfs/dbuf.c: In function 'dbuf_destroy': module/zfs/dbuf.c:2953:17: error: pointer 'db' may be used after 'free' [-Werror=use-after-free] Reviewed-by: Ryan Moeller <ryan@iXsystems.com> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#13528 Closes openzfs#13575

Extend the buffer slightly resolve the warning. cmd/zfs/zfs_main.c: In function ‘upgrade_set_callback’: cmd/zfs/zfs_main.c:2446:22: error: ‘%llu’ directive output may be truncated writing between 1 and 20 bytes into a region of size 16 [-Werror=format-truncation=] cmd/zfs/zfs_main.c:2445:24: note: ‘snprintf’ output between 2 and 21 bytes into a destination of size 16 Reviewed-by: Ryan Moeller <ryan@iXsystems.com> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#13528 Closes openzfs#13575

Switch to using asprintf() to satisfy the compiler and resolve the potential format-overflow warning. Not the conditional before the sprintf() would have prevented this regardless. cmd/zfs/zfs_project.c: In function ‘zfs_project_handle_dir’: cmd/zfs/zfs_project.c:241:38: error: ‘/’ directive writing 1 byte into a region of size between 0 and 4352 [-Werror=format-overflow=] cmd/zfs/zfs_project.c:241:17: note: ‘sprintf’ output between 2 and 4609 bytes into a destination of size 4352 Reviewed-by: Ryan Moeller <ryan@iXsystems.com> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#13528 Closes openzfs#13575

Resolve straight-line speculation warnings reported by objtool for x86_64 assembly on Linux when CONFIG_SLS is set. See the following LWN article for the complete details. https://lwn.net/Articles/877845/ Reviewed-by: Ryan Moeller <ryan@iXsystems.com> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#13528 Closes openzfs#13575

This code should be kept inline with the upstream lua version as much as possible. Therefore, we simply want to silence the warning. This check was enabled by default as part of -Wall in gcc 12.1. Reviewed-by: Ryan Moeller <ryan@iXsystems.com> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#13528 Closes openzfs#13575

Restructure the code in zfs_log_xvattr() to use a lr_attr_end structure when accessing lr_attr_t elements located after the variable sized array. This makes the code more understandable and resolves the accessing beyond the end of the field warnings. Reviewed-by: Ryan Moeller <ryan@iXsystems.com> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#13528 Closes openzfs#13575