linux-kernel - Re: [PATCH 1/1] Net: bcm.c: Remove Subtree Instead of Entry

lists.openwall.net		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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2bf44b8d-b286-4a94-8e1d-6c4e736a1d07@hartkopp.net>
Date: Fri, 9 Aug 2024 11:57:41 +0200
From: Oliver Hartkopp <socketcan@...tkopp.net>
To: David Hunter <david.hunter.linux@...il.com>, mkl@...gutronix.de,
 davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, linux-can@...r.kernel.org, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org
Cc: skhan@...uxfoundation.org, javier.carrasco.cruz@...il.com
Subject: Re: [PATCH 1/1] Net: bcm.c: Remove Subtree Instead of Entry

Hello David,

many thanks for the patch and the description.

Btw. the data structures of the elements inside that bcm proc dir should 
have been removed at that point, so that the can-bcm dir should be empty.

I'm not sure what happens to the open sockets that are (later) removed 
in bcm_release() when we use remove_proc_subtree() as suggested. 
Removing this warning probably does not heal the root cause of the issue.

What did you do to trigger the warning? Did you work with network 
namespaces or LXC/Docker and purged an entire namespace?

Best regards,
Oliver

On 08.08.24 22:26, David Hunter wrote:
> Fix a warning with bcm.c that is caused by removing an entry. If the
> entry had a process as a child, a warning is generated:
> 
> remove_proc_entry: removing non-empty directory 'net/can-bcm'...
> WARNING: CPU: 1 PID: 71 at fs/proc/generic.c:717 remove_proc_entry
> Call Trace:
> remove_proc_entry
> canbcm_pernet_exit
> ops_exit_list
> 
> Instead of simply removing the entry, remove the entire subdirectory.
> The child process will still be removed, but without a warning occurring.
> 
> This patch was compiled and the code traced with gdb to see that the
> tree  was removed. The code was run to see that the warning was removed.
> In addition, the code was tested with the kselftest
> net subsystem. No regressions were detected.
> 
> Signed-off-by: David Hunter <david.hunter.linux@...il.com>
> ---
>   net/can/bcm.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 27d5fcf0eac9..fea48fd793e5 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -1779,7 +1779,7 @@ static void canbcm_pernet_exit(struct net *net)
>   #if IS_ENABLED(CONFIG_PROC_FS)
>   	/* remove /proc/net/can-bcm directory */
>   	if (net->can.bcmproc_dir)
> -		remove_proc_entry("can-bcm", net->proc_net);
> +		remove_proc_subtree("can-bcm", net->proc_net);
>   #endif /* CONFIG_PROC_FS */
>   }
>