linux-kernel - Re: Future handling of complex RGB devices on Linux v2

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: <b6d79727-ae94-44b1-aa88-069416435c14@redhat.com>
Date: Thu, 22 Feb 2024 11:46:03 +0100
From: Hans de Goede <hdegoede@...hat.com>
To: Pavel Machek <pavel@....cz>, Werner Sembach <wse@...edocomputers.com>
Cc: Lee Jones <lee@...nel.org>, jikos@...nel.org,
 linux-kernel@...r.kernel.org, Jelle van der Waa <jelle@...aa.nl>,
 Miguel Ojeda <miguel.ojeda.sandonis@...il.com>,
 "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
 linux-input@...r.kernel.org, ojeda@...nel.org, linux-leds@...r.kernel.org
Subject: Re: Future handling of complex RGB devices on Linux v2

Hi,

On 2/21/24 23:17, Pavel Machek wrote:
> Hi!
> 
>> so after more feedback from the OpenRGB maintainers I came up with an even
>> more generic proposal:
>> https://gitlab.com/CalcProgrammer1/OpenRGB/-/issues/3916#note_1753072869
> 
>>> evaluate-set-command ioctl taking:
>>> {
>>>     enum command                /* one of supported_commands */
>>>     union data
>>>     {
>>>         char raw[3072],
>>>         {
>>>             <input struct for command 0>
>>>         },
> 
> Yeah, so ... this is not a interface. This is a backdoor to pass
> arbitrary data. That's not going to fly.

Pavel, Note the data will be interpreted by a kernel driver and
not passed directly to the hw.

With that said I tend to agree that this seems to be a bit too
generic.

Werner, it seems you are basically re-inventing ioctls here.

If you are going to use per vendor specific data structs for various
commands and have those defined in the kernel userspace API headers,
then this means that userspace will already need updated versions
of those headers to support new vendors / new laptop models if
the commands change for a new model.

So what you are basically providing here is a generic interface
to pass a cmd number + a cmd-bumber-specific data struct and
we already have that it is called an ioctl.

So I think that the conclusion of this whole discussion is that
with the exception of a get-dev-info ioctl, we simply want
vendor specific ioctls, using 1 ioctl per command.

Given that these devices are all different in various ways
and that we only want this for devices which cannot be accessed
from userspace directly (so a limit set of devices) I really
think that just doing custom ioctls per vendor is best.

This certainly is the most KISS approach. This proposal
in essence is just an arbitrary command multiplexer /
demultiplexer and ioctls already are exactly that.

With the added advantage of being able to directly use
pass the vendor-cmd-specific struct to the ioctl instead
of having to first embed it in some other struct.

Regards,

Hans