[PATCH][next] cgroup: Avoid -Wstringop-overflow warnings

[PATCH][next] cgroup: Avoid -Wstringop-overflow warnings

[Gustavo A. R. Silva]

Address the following -Wstringop-overflow warnings seen when
built with ARM architecture and aspeed_g4_defconfig configuration
(notice that under this configuration CGROUP_SUBSYS_COUNT == 0):
kernel/cgroup/cgroup.c:1208:16: warning: 'find_existing_css_set' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
kernel/cgroup/cgroup.c:1258:15: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
kernel/cgroup/cgroup.c:6089:18: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
kernel/cgroup/cgroup.c:6153:18: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]

These changes are based on commit d20d30ebb199 ("cgroup: Avoid compiler
warnings with no subsystems").

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 kernel/cgroup/cgroup.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index cd497b90e11a..1ee76e62eb98 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1200,6 +1200,9 @@ static struct css_set *find_css_set(struct css_set *old_cset,
 	unsigned long key;
 	int ssid;
 
+	if (!CGROUP_HAS_SUBSYS_CONFIG)
+		return NULL;
+
 	lockdep_assert_held(&cgroup_mutex);
 
 	/* First see if we already have a cgroup group that matches
@@ -6045,6 +6048,9 @@ int __init cgroup_init(void)
 	struct cgroup_subsys *ss;
 	int ssid;
 
+	if (!CGROUP_HAS_SUBSYS_CONFIG)
+		return -EINVAL;
+
 	BUILD_BUG_ON(CGROUP_SUBSYS_COUNT > 16);
 	BUG_ON(cgroup_init_cftypes(NULL, cgroup_base_files));
 	BUG_ON(cgroup_init_cftypes(NULL, cgroup_psi_files));
-- 
2.34.1

Re: [PATCH][next] cgroup: Avoid -Wstringop-overflow warnings

[Kees Cook]

On Wed, Jun 14, 2023 at 07:18:22PM -0600, Gustavo A. R. Silva wrote:
> Address the following -Wstringop-overflow warnings seen when
> built with ARM architecture and aspeed_g4_defconfig configuration
> (notice that under this configuration CGROUP_SUBSYS_COUNT == 0):
> kernel/cgroup/cgroup.c:1208:16: warning: 'find_existing_css_set' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> kernel/cgroup/cgroup.c:1258:15: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> kernel/cgroup/cgroup.c:6089:18: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> kernel/cgroup/cgroup.c:6153:18: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> 
> These changes are based on commit d20d30ebb199 ("cgroup: Avoid compiler
> warnings with no subsystems").
> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Ah nice, this should reduce code size for the "0" case too.

Reviewed-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

-- 
Kees Cook

Re: [PATCH][next] cgroup: Avoid -Wstringop-overflow warnings

[Tejun Heo]

On Wed, Jun 14, 2023 at 07:18:22PM -0600, Gustavo A. R. Silva wrote:
> Address the following -Wstringop-overflow warnings seen when
> built with ARM architecture and aspeed_g4_defconfig configuration
> (notice that under this configuration CGROUP_SUBSYS_COUNT == 0):
> kernel/cgroup/cgroup.c:1208:16: warning: 'find_existing_css_set' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> kernel/cgroup/cgroup.c:1258:15: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> kernel/cgroup/cgroup.c:6089:18: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> kernel/cgroup/cgroup.c:6153:18: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> 
> These changes are based on commit d20d30ebb199 ("cgroup: Avoid compiler
> warnings with no subsystems").
> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Applied to cgroup/for-6.5.

Thanks.

-- 
tejun

Re: [PATCH][next] cgroup: Avoid -Wstringop-overflow warnings

[Steven Price]

On 15/06/2023 02:18, Gustavo A. R. Silva wrote:
> Address the following -Wstringop-overflow warnings seen when
> built with ARM architecture and aspeed_g4_defconfig configuration
> (notice that under this configuration CGROUP_SUBSYS_COUNT == 0):
> kernel/cgroup/cgroup.c:1208:16: warning: 'find_existing_css_set' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> kernel/cgroup/cgroup.c:1258:15: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> kernel/cgroup/cgroup.c:6089:18: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> kernel/cgroup/cgroup.c:6153:18: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> 
> These changes are based on commit d20d30ebb199 ("cgroup: Avoid compiler
> warnings with no subsystems").
> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  kernel/cgroup/cgroup.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index cd497b90e11a..1ee76e62eb98 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -1200,6 +1200,9 @@ static struct css_set *find_css_set(struct css_set *old_cset,
>  	unsigned long key;
>  	int ssid;
>  
> +	if (!CGROUP_HAS_SUBSYS_CONFIG)
> +		return NULL;
> +
>  	lockdep_assert_held(&cgroup_mutex);
>  
>  	/* First see if we already have a cgroup group that matches
> @@ -6045,6 +6048,9 @@ int __init cgroup_init(void)
>  	struct cgroup_subsys *ss;
>  	int ssid;
>  
> +	if (!CGROUP_HAS_SUBSYS_CONFIG)
> +		return -EINVAL;
> +
>  	BUILD_BUG_ON(CGROUP_SUBSYS_COUNT > 16);
>  	BUG_ON(cgroup_init_cftypes(NULL, cgroup_base_files));
>  	BUG_ON(cgroup_init_cftypes(NULL, cgroup_psi_files));

This change (which landed in linux-next) causes a boot failure on my
(arm32) board because the cgroup filesystem isn't created which upsets
systemd:

[   11.474767] systemd[1]: Failed to mount tmpfs at /sys/fs/cgroup: No such file or directory
[   11.489933] systemd[1]: Failed to mount cgroup at /sys/fs/cgroup/systemd: No such file or directory
[!!!!!!] Failed to mount API filesystems.

Reverting this commit on the head of linux-next gets the board working
again.

Thanks,

Steve

Re: [PATCH][next] cgroup: Avoid -Wstringop-overflow warnings

[Krzysztof Kozlowski]

On 15/06/2023 03:18, Gustavo A. R. Silva wrote:
> Address the following -Wstringop-overflow warnings seen when
> built with ARM architecture and aspeed_g4_defconfig configuration
> (notice that under this configuration CGROUP_SUBSYS_COUNT == 0):
> kernel/cgroup/cgroup.c:1208:16: warning: 'find_existing_css_set' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> kernel/cgroup/cgroup.c:1258:15: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> kernel/cgroup/cgroup.c:6089:18: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> kernel/cgroup/cgroup.c:6153:18: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> 

Hi,

This patch landed in next but causes boot failures on
Exynos ARMv7 board (32 bit) with systemd (exynos_defconfig):

NFS-Mount: 192.168.1.10:/srv/nfs/odroidhc1
Waiting 10 seconds for device /dev/nfs ...
ERROR: device '/dev/nfs' not found. Skipping fsck.
Mount cmd: 
mount.nfs4 -o vers=4,nolock 192.168.1.10:/srv/nfs/odroidhc1 /new_root
:: running cleanup hook [udev]
[   23.752917] systemd[1]: System time before build time, advancing clock.
[   23.851828] systemd[1]: Failed to mount tmpfs at /sys/fs/cgroup: No such file or directory
[   23.868459] systemd[1]: Failed to mount cgroup at /sys/fs/cgroup/systemd: No such file or directory
[!!!!!!] Failed to mount API filesystems.
[   23.914562] systemd[1]: Freezing execution.

Full log:
https://krzk.eu/#/builders/21/builds/4110/steps/15/logs/serial0

Best regards,
Krzysztof

Re: [PATCH][next] cgroup: Avoid -Wstringop-overflow warnings

[Gustavo A. R. Silva]

Hi!

On 6/22/23 04:11, Steven Price wrote:
> On 15/06/2023 02:18, Gustavo A. R. Silva wrote:
>> Address the following -Wstringop-overflow warnings seen when
>> built with ARM architecture and aspeed_g4_defconfig configuration
>> (notice that under this configuration CGROUP_SUBSYS_COUNT == 0):
>> kernel/cgroup/cgroup.c:1208:16: warning: 'find_existing_css_set' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
>> kernel/cgroup/cgroup.c:1258:15: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
>> kernel/cgroup/cgroup.c:6089:18: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
>> kernel/cgroup/cgroup.c:6153:18: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
>>
>> These changes are based on commit d20d30ebb199 ("cgroup: Avoid compiler
>> warnings with no subsystems").
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>> ---
>>   kernel/cgroup/cgroup.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
>> index cd497b90e11a..1ee76e62eb98 100644
>> --- a/kernel/cgroup/cgroup.c
>> +++ b/kernel/cgroup/cgroup.c
>> @@ -1200,6 +1200,9 @@ static struct css_set *find_css_set(struct css_set *old_cset,
>>   	unsigned long key;
>>   	int ssid;
>>   
>> +	if (!CGROUP_HAS_SUBSYS_CONFIG)
>> +		return NULL;
>> +
>>   	lockdep_assert_held(&cgroup_mutex);
>>   
>>   	/* First see if we already have a cgroup group that matches
>> @@ -6045,6 +6048,9 @@ int __init cgroup_init(void)
>>   	struct cgroup_subsys *ss;
>>   	int ssid;
>>   
>> +	if (!CGROUP_HAS_SUBSYS_CONFIG)
>> +		return -EINVAL;
>> +
>>   	BUILD_BUG_ON(CGROUP_SUBSYS_COUNT > 16);
>>   	BUG_ON(cgroup_init_cftypes(NULL, cgroup_base_files));
>>   	BUG_ON(cgroup_init_cftypes(NULL, cgroup_psi_files));
> 
> This change (which landed in linux-next) causes a boot failure on my
> (arm32) board because the cgroup filesystem isn't created which upsets
> systemd:
> 
> [   11.474767] systemd[1]: Failed to mount tmpfs at /sys/fs/cgroup: No such file or directory
> [   11.489933] systemd[1]: Failed to mount cgroup at /sys/fs/cgroup/systemd: No such file or directory
> [!!!!!!] Failed to mount API filesystems.
> 
> Reverting this commit on the head of linux-next gets the board working
> again.

Thanks for reporting this issue. I'll take a look.

--
Gustavo

Re: [PATCH][next] cgroup: Avoid -Wstringop-overflow warnings

[Gustavo A. R. Silva]

Hi!

On 6/22/23 04:20, Krzysztof Kozlowski wrote:
> On 15/06/2023 03:18, Gustavo A. R. Silva wrote:
>> Address the following -Wstringop-overflow warnings seen when
>> built with ARM architecture and aspeed_g4_defconfig configuration
>> (notice that under this configuration CGROUP_SUBSYS_COUNT == 0):
>> kernel/cgroup/cgroup.c:1208:16: warning: 'find_existing_css_set' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
>> kernel/cgroup/cgroup.c:1258:15: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
>> kernel/cgroup/cgroup.c:6089:18: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
>> kernel/cgroup/cgroup.c:6153:18: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
>>
> 
> Hi,
> 
> This patch landed in next but causes boot failures on
> Exynos ARMv7 board (32 bit) with systemd (exynos_defconfig):
> 
> NFS-Mount: 192.168.1.10:/srv/nfs/odroidhc1
> Waiting 10 seconds for device /dev/nfs ...
> ERROR: device '/dev/nfs' not found. Skipping fsck.
> Mount cmd:
> mount.nfs4 -o vers=4,nolock 192.168.1.10:/srv/nfs/odroidhc1 /new_root
> :: running cleanup hook [udev]
> [   23.752917] systemd[1]: System time before build time, advancing clock.
> [   23.851828] systemd[1]: Failed to mount tmpfs at /sys/fs/cgroup: No such file or directory
> [   23.868459] systemd[1]: Failed to mount cgroup at /sys/fs/cgroup/systemd: No such file or directory
> [!!!!!!] Failed to mount API filesystems.
> [   23.914562] systemd[1]: Freezing execution.
> 
> Full log:
> https://krzk.eu/#/builders/21/builds/4110/steps/15/logs/serial0

Thanks for reporting this issue. I'll take a look.

Re: [PATCH][next] cgroup: Avoid -Wstringop-overflow warnings

[Tejun Heo]

On Thu, Jun 22, 2023 at 08:09:03AM -0600, Gustavo A. R. Silva wrote:
> > Reverting this commit on the head of linux-next gets the board working
> > again.
> 
> Thanks for reporting this issue. I'll take a look.

I'm reverting the commit from cgroup/for-6.5. Let's try again later.

Thanks.

-- 
tejun

Re: [PATCH][next] cgroup: Avoid -Wstringop-overflow warnings

[Gustavo A. R. Silva]

Hi all,

I wonder if you have any suggestions on how to address this issue. As it seems that
my last attempt caused some boot failures[1][2].

At first, I thought that the right way to fix this was through a similar fix as this
one[3]. But it seems I'm missing something else that I cannot determine yet.

These -Wstringop-overflow warnings are mostly the last ones remaining before we can
finally enable this compiler option, globally.

Any help or advice on how to properly address this is greatly appreciated. :)

Thanks!
--
Gustavo

[1] https://lore.kernel.org/linux-hardening/726aae97-755d-9806-11d4-2fb21aa93428@arm.com/
[2] https://lore.kernel.org/linux-hardening/361c2f87-1424-f452-912f-0e4a339f5c46@kernel.org/
[3] https://git.kernel.org/linus/d20d30ebb199


On 6/14/23 19:18, Gustavo A. R. Silva wrote:
> Address the following -Wstringop-overflow warnings seen when
> built with ARM architecture and aspeed_g4_defconfig configuration
> (notice that under this configuration CGROUP_SUBSYS_COUNT == 0):
> kernel/cgroup/cgroup.c:1208:16: warning: 'find_existing_css_set' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> kernel/cgroup/cgroup.c:1258:15: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> kernel/cgroup/cgroup.c:6089:18: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> kernel/cgroup/cgroup.c:6153:18: warning: 'css_set_hash' accessing 4 bytes in a region of size 0 [-Wstringop-overflow=]
> 
> These changes are based on commit d20d30ebb199 ("cgroup: Avoid compiler
> warnings with no subsystems").
> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>   kernel/cgroup/cgroup.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index cd497b90e11a..1ee76e62eb98 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -1200,6 +1200,9 @@ static struct css_set *find_css_set(struct css_set *old_cset,
>   	unsigned long key;
>   	int ssid;
>   
> +	if (!CGROUP_HAS_SUBSYS_CONFIG)
> +		return NULL;
> +
>   	lockdep_assert_held(&cgroup_mutex);
>   
>   	/* First see if we already have a cgroup group that matches
> @@ -6045,6 +6048,9 @@ int __init cgroup_init(void)
>   	struct cgroup_subsys *ss;
>   	int ssid;
>   
> +	if (!CGROUP_HAS_SUBSYS_CONFIG)
> +		return -EINVAL;
> +
>   	BUILD_BUG_ON(CGROUP_SUBSYS_COUNT > 16);
>   	BUG_ON(cgroup_init_cftypes(NULL, cgroup_base_files));
>   	BUG_ON(cgroup_init_cftypes(NULL, cgroup_psi_files));

Re: [PATCH][next] cgroup: Avoid -Wstringop-overflow warnings

[Tejun Heo]

On Tue, Aug 15, 2023 at 01:04:32PM -0600, Gustavo A. R. Silva wrote:
> Hi all,
> 
> I wonder if you have any suggestions on how to address this issue. As it seems that
> my last attempt caused some boot failures[1][2].
> 
> At first, I thought that the right way to fix this was through a similar fix as this
> one[3]. But it seems I'm missing something else that I cannot determine yet.
> 
> These -Wstringop-overflow warnings are mostly the last ones remaining before we can
> finally enable this compiler option, globally.
> 
> Any help or advice on how to properly address this is greatly appreciated. :)

It looks like there are only two functions which misbehave when there are no
controllers configured. Maybe just fix them so that they don't access empty
array?

Thanks.

-- 
tejun