linux-kernel - Re: [PATCH v1] hid-playstation: DS4: Update rumble and lightbar together

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: <dedb2c39-fc28-4cba-802f-5d56f23db722@enpas.org>
Date: Fri, 21 Jun 2024 04:26:26 +0900
From: Max Staudt <max@...as.org>
To: Roderick Colenbrander <thunderbird2k@...il.com>
Cc: Roderick Colenbrander <roderick.colenbrander@...y.com>,
 Jiri Kosina <jikos@...nel.org>,
 Benjamin Tissoires <benjamin.tissoires@...hat.com>,
 linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] hid-playstation: DS4: Update rumble and lightbar
 together

Hi Roderick,

So as far as I understand, my suggested driver behaviour is sound, because both the console's own behaviour as well as other drivers show that flags == 0x03 is working perfectly fine with original controllers. Is my understanding correct?

In fact, hid-sony used to send these updates at the same time (it had flags == 0x07), so for some 3rd party controllers, the move to hid-playstation has already been a regression in the FF/lightbar department.

Do you see a way to move forward with this patch and get it merged, even if it is with some delay? Is there something that I can improve?

As for downstream users' regression tests, this argument confuses me. Could you please give me a bit of help here?

My understanding, so far, is as follows:

Tests checking the FF bit should not fail if, say, the lightbar bit is also set. If they fail, then that means that the test is too sensitive. After all, the patch does not change anything from userspace's point of view, nor from the actual human user's point of view. The DualShock 4 behaves all the same, and it's just the wire protocol that changes.

So if a downstream user wishes to do a full end-to-end integration test, technically they would need to connect a real DualShock 4 and test that. But I can see that this is not easily automatable ;) so they may test at the HID level instead. The result is that, depending on how they structure their tests, they might no longer be testing end-to-end, but rather testing an implementation and its quirks. This bears the risk that the test will fail because of a legitimate change in the driver, or elsewhere in the kernel.

I suppose this is what you want to avoid, but... isn't avoiding such changes the reason why LTS kernels exist?

And there is only one LTS kernel with this driver, v6.6, released 8 months ago. How did it become necessary to ossify the driver's wire behaviour in this time frame?

Hence I'm confused why changing the wire protocol upstream, without breaking any functionality on the original controller or backporting to LTS kernels, creates problems. Either the tests are correctly written in a way to not be affected by this change, mimicking the original gamepad which is unaffected. Or the tests are protected from breaking by using LTS kernels. In the latter case, the tests will break on a kernel version bump - legitimately so, and fixing them should be easy.

Am I missing something?

Do you see a way to get this patch in?

Would it help you to have some time for warning your downstream contacts to stabilise their tests, and I could re-send this patch in 6 months from now?

Hopeful greetings,
Max