netdev - Re: [PATCH net-next v7 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] [day] [month] [year] [list]

Message-ID: <CANBPYPjq5BC3t-kF2Kcbs4tdCd=Zzd4Qk3mzRg-qyda3tEE3sA@mail.gmail.com>
Date: Tue, 5 Nov 2024 13:00:08 -0800
From: Li Li <dualli@...omium.org>
To: Jakub Kicinski <kuba@...nel.org>
Cc: 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, 
	cmllamas@...gle.com, 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 net-next v7 2/2] binder: report txn errors via generic netlink

On Mon, Nov 4, 2024 at 6:41 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Mon, 4 Nov 2024 09:12:37 -0800 Li Li wrote:
> > That's why binder genl uses unicast instead of multicast. The administration
> > process of the OS (Android in this case) always runs before any other user
> > applications, which registers itself to the kernel binder driver and uses it
> > exclusively. With a unified family name, the same userspace admin process
> > has access to all binder contexts. With separate family names, each domain
> > admin process can register itself to the corresponding binder context.
>
> Side note - it'd be useful for my understanding to know what the binder
> families would be. You register them in binderfs_binder_device_create(),
> what creates the devices and how many do we expect?

There are 2 ways to create a new binder device.
1. Provide a list of devices (as strings) in boot cmd;
2. Call an API provided by binderfs in the system init process.

There are 3 domains as mentioned at
https://source.android.com/docs/core/architecture/hidl/binder-ipc#ipc

>
> Back to answering - I don't know why the same process would have to bind
> to all domains. You record which portid is bound to the context, the portid
> identifies the socket.
>
> BTW portids can get reused, do you need to do something when the admin
> process dies? To prevent normal user from binding to the same portid?

When the admin process dies, the kernel driver automatically releases the
binder fd and resets the netlink. So a normal user won't be able to bind
to the same portid. The admin process will respawn itself and re-initialize
everything.

>
> > So, do you think the current implementation of registering multiple families
> > with different names acceptable? Or is there a better way to do it? Thank
> > you very much!
>
> I don't see what having the separate families buys you.
> The genl family ID is added to the netlink message in nlmsg_type.
> That's it, it doesn't do anything else.
> You can add an attribute to the message to carry context->name
> and you'll be able to demux the messages.

You're right. Even though my userspace code uses different sockets to talk
to different binder genl families, the generic netlink socket doesn't enforce
this. Other apps can still use one socket to talk to different families.

I'll add a new attribute to demux the message, which should be able to achieve
the same functionality.