add CONFIG_F2FS_FS_SECURITY=y

[vch42]

I think the CONFIG_F2FS_FS_SECURITY=y option should be enabled in the kernel.

It enables support for the XATTR security namespace, which atm is not supported due to this option currently not being set to yes.
The xattr security namespace is used by SELinux and is also a requirement for Samba4 AD/DC full support.

We already have XATTR and POSIX_ACL support enabled for F2FS, with user namespace support, it only makes sense to fully support XATTR on F2FS by also enabling CONFIG_F2FS_FS_SECURITY.

XATTR F2FS support added following: #2508

Thank you.

[JamesH65]

Do you have any numbers for the impact this has on memory requriements and overall CPU performance impact?

[vch42]

I don't have a comparative test for the memory usage and CPU performance with and without the feature enabled in the kernel, but I have a Pi with this enabled, which did not had it prior, and I do not see a noticeable difference in feel.
Sure, this is only based on "feel" and my own impression on how the device behaves. It is in no way an exact test. Plus, I am not very sure how I can produce such numbers, I am not very versed in such benchmarking on Linux.
However, not enabling the XATTR security namespace, but only the user namespace, leaves a lot of the uses of XATTR out of the picture due to lack of support. This increases the uselessness level of the previous activation of CONFIG_F2FS_FS_XATTR in the kernel.

[JamesH65]

Did a quick recompile with this enabled - adds 352 bytes on the Pi4 zImage. Looking at the code, cannot see if having much of a effect on the performance, and it's only used when f2fs is being use I believe. So seems like an OK addition.

[JamesH65]

@popcornmix @pelwell

[popcornmix]

Okay.

[JamesH65]

@dom Not yet enabled - do we intend to do so?

If f2fs is intended to be usable for root filesystem, it makes sense for it to have feature parity with ext4 where possible. Currently, CONFIG_F2FS_FS_SECURITY appears to be the only config option missing:

# grep -C0 -e F2FS -e EXT4 /usr/src/linux-headers-5.10.17-v8+/.config 
CONFIG_EXT4_FS=y
CONFIG_EXT4_USE_FOR_EXT2=y
CONFIG_EXT4_FS_POSIX_ACL=y
CONFIG_EXT4_FS_SECURITY=y
# CONFIG_EXT4_DEBUG is not set
--
CONFIG_F2FS_FS=y
CONFIG_F2FS_STAT_FS=y
CONFIG_F2FS_FS_XATTR=y
CONFIG_F2FS_FS_POSIX_ACL=y
# CONFIG_F2FS_FS_SECURITY is not set
# CONFIG_F2FS_CHECK_FS is not set
# CONFIG_F2FS_IO_TRACE is not set
# CONFIG_F2FS_FAULT_INJECTION is not set
# CONFIG_F2FS_FS_COMPRESSION is not set

As a specific example, F2FS_FS_SECURITY enables the use of setcap to grant capabilities to binaries (such as CAP_NET_RAW) instead of requiring them to be suid-root. This is used by the default-installed ping (if you update/--reinstall; I guess the imaging process doesn't support capabilities?):

# getcap /bin/* /sbin/*
/bin/fping = cap_net_raw+ep
/bin/mtr-packet = cap_net_raw+ep
/bin/ping = cap_net_raw+ep

[pelwell]

This does indeed add very little code:

f2fs_init_inode_metadata 518 -> 530 (+18)
f2fs_add_regular_entry 5ec -> 5e8 (-4)
f2fs_hash_filename 24c -> 248 (-4)
f2fs_init_security 0 -> 34 (+34)
f2fs_initxattrs 0 -> 7c (+7c)

In fact the extra seems to get swallowed up in such a way that the memory map doesn't expand.

ext4 has exactly the same initialisation hook, but it configures a different set of attributes.

Since the code size change is negligible, and any (presumably tiny) runtime overhead only affects F2FS users (who might benefit from the change) I'm prepared to enable the config setting.

[pelwell]

See #4637.