diff mbox series

[next] cgroup: Avoid -Wstringop-overflow warnings

Message ID ZIpm3pcs3iCP9UaR@work (mailing list archive)
State Mainlined
Commit 36de5f303ca1bd6fce74815ef17ef3d8ff8737b5
Headers show
Series [next] cgroup: Avoid -Wstringop-overflow warnings | expand

Commit Message

Gustavo A. R. Silva June 15, 2023, 1:18 a.m. UTC
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(+)

Comments

Kees Cook June 20, 2023, 7:51 p.m. UTC | #1
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@kernel.org>

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

Reviewed-by: Kees Cook <keescook@chromium.org>
Tejun Heo June 21, 2023, 8:42 p.m. UTC | #2
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@kernel.org>

Applied to cgroup/for-6.5.

Thanks.
Steven Price June 22, 2023, 10:11 a.m. UTC | #3
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,

Steve
Krzysztof Kozlowski June 22, 2023, 10:20 a.m. UTC | #4
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
Gustavo A. R. Silva June 22, 2023, 2:09 p.m. UTC | #5
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
Gustavo A. R. Silva June 22, 2023, 2:09 p.m. UTC | #6
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.

--
Gustavo
Tejun Heo June 22, 2023, 6:50 p.m. UTC | #7
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.
Gustavo A. R. Silva Aug. 15, 2023, 7:04 p.m. UTC | #8
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));
Tejun Heo Aug. 16, 2023, 7:22 p.m. UTC | #9
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.
diff mbox series

Patch

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));