netdev - Re: [PATCH v11 2/2] binder: report txn errors via generic netlink

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: <CANBPYPjQVqmzZ4J=rVQX87a9iuwmaetULwbK_5_3YWk2eGzkaA@mail.gmail.com>
Date: Mon, 13 Jan 2025 22:01:00 -0800
From: Li Li <dualli@...omium.org>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Carlos Llamas <cmllamas@...gle.com>, dualli@...gle.com, corbet@....net, 
	davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com, 
	donald.hunter@...il.com, gregkh@...uxfoundation.org, arve@...roid.com, 
	tkjos@...roid.com, maco@...roid.com, joel@...lfernandes.org, 
	brauner@...nel.org, surenb@...gle.com, arnd@...db.de, masahiroy@...nel.org, 
	bagasdotme@...il.com, horms@...nel.org, linux-kernel@...r.kernel.org, 
	linux-doc@...r.kernel.org, netdev@...r.kernel.org, hridya@...gle.com, 
	smoreland@...gle.com, kernel-team@...roid.com
Subject: Re: [PATCH v11 2/2] binder: report txn errors via generic netlink

On Thu, Jan 9, 2025 at 4:18 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Thu, 9 Jan 2025 23:19:40 +0000 Carlos Llamas wrote:
> > On Thu, Jan 09, 2025 at 12:13:00PM -0800, Jakub Kicinski wrote:
> > > On Thu, 9 Jan 2025 11:48:24 -0800 Li Li wrote:
> > > > Cleaning up in the NETLINK_URELEASE notifier is better since we
> > > > register the process with the netlink socket. I'll change the code
> > > > accordingly.
> > >
> > > Hm. Thought I already told you this. Maybe I'm mixing up submissions.
> > >
> > > Please the unbind callback or possibly the sock priv infra
> > > (genl_sk_priv_get, sock_priv_destroy etc).
> >
> > Sorry, it was me that suggested NETLINK_URELEASE. BTW, I did try those
> > genl_family callbacks first but I couldn't get them to work right away
> > so I moved on. I'll have a closer look now to figure out what I did
> > wrong. Thanks for the suggestion Jakub!
>
> Hm, that's probably because there is no real multicast group here :(
> genl_sk_priv_get() and co. may work better in that case.
> your suggestion of NETLINK_URELEASE may work too, tho, I think it's
> the most error prone

sock_priv_destroy works with genl_sk_priv_get().

But, I have to manually modify the generated netlink header file to satisfy CFI.

-void binder_nl_sock_priv_init(struct my_struct *priv);
-void binder_nl_sock_priv_destroy(struct my_struct *priv);
+void binder_nl_sock_priv_init(void *priv);
+void binder_nl_sock_priv_destroy(void *priv);

The reason is that these 2 callback functions are defined in
include/net/genetlink.h as below
void (*sock_priv_init)(void *priv);
void (*sock_priv_destroy)(void *priv);

Otherwise, kernel panic when CFI is enabled.

CFI failure at genl_sk_priv_get+0x60/0x138 (target:
binder_nl_sock_priv_init+0x0/0x34; expected type: 0x0ef81b7d)

Jakub, we probably need this patch. Please let me know if you have a
better idea. Thanks!

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 8155ff6d2a38..84033938a75f 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -2352,8 +2352,8 @@ def print_kernel_family_struct_hdr(family, cw):
     cw.p(f"extern struct genl_family {family.c_name}_nl_family;")
     cw.nl()
     if 'sock-priv' in family.kernel_family:
-        cw.p(f'void
{family.c_name}_nl_sock_priv_init({family.kernel_family["sock-priv"]}
*priv);')
-        cw.p(f'void
{family.c_name}_nl_sock_priv_destroy({family.kernel_family["sock-priv"]}
*priv);')
+        cw.p(f'void {family.c_name}_nl_sock_priv_init(void *priv);')
+        cw.p(f'void {family.c_name}_nl_sock_priv_destroy(void *priv);')
         cw.nl()