Re: [PATCH] HID: sony: Fix division by zero

[Roderick Colenbrander <thunderbird2k@gmail.com>]

Hi Alain,

Thanks for testing. I added a similar patch to my hid-playstation tree
this morning (hid-sony will go away soon).

I'm not entirely happy with the patches yet and need to do some
thinking. The issue is the value range, which is not correct. For the
accelerometer the numerator and denominator need to be 1 to match the
proper range. (It just happens that the scaling factors are the way
they are I think.)

The gyroscope is the issue and I'm not entirely sure what the
numerator needs to be. If I print the coefficients using one of my
dualshocks, the ratio of numerator to denominator is around 60. And
then I'm hunted by some comment from someone in the community..
https://dsremap.readthedocs.io/en/latest/reverse.html#use-calibration

"Note:
There’s a 2 factor in the Linux driver, i.e. max_speed and min_speed
are added instead of averaged. Either there’s something I don’t get,
or the factor is taken care of in the resolution constant, or it’s a
bug.
"

I need to do some thinking on whether the current code is right (even
if it isn't, it can't be changed as it would break software). Then
what the factor needs to be.

Thanks,
Roderick

On Fri, Dec 30, 2022 at 10:12 AM Alain <alain.carlucci@gmail.com> wrote:
>
> Hi Roderick,
> Hi Daniel,
>
> Thank you for all the suggestions, Roderick.
> I fixed the typo in your patch, backported to hid-sony and tested both of them.
> They fix the issue and the DS4 can be used even without calibration data.
> I cannot test if everything works well with a DualSense because I do not own one.
>
> Hi Daniel, I've seen that in the hid-nintendo driver, the calibration denominator is not sanitized if it's zero.
> If I'm not mistaken, a device that disguises itself as a joycon with denominator zero can crash the kernel
> when this is used in the mult_frac(). I had this behaviour with the hid-sony driver.
> You can find attached the patch that should solve the problem.
>
> Thanks,
> Alain
>
>
> Il giorno gio 29 dic 2022 alle ore 21:56 Roderick Colenbrander <thunderbird2k@gmail.com> ha scritto:
>>
>> Hi Alain,
>>
>> On Thu, Dec 29, 2022 at 11:21 AM Alain Carlucci
>> <alain.carlucci@gmail.com> wrote:
>> >
>> > Hi Roderick,
>> >
>> > >Give this patch a try. It is against hid-playstation, which as of 6.2
>> > >supports DS4. Let me know if it works. You can see the sensors working
>> > >through evdev on the motion sensors device.
>> >
>> > Thank you for the patch, just tried. I think there is a typo in the
>> > condition of the for-loop that sanitizes the input.
>> > Instead of `; i < ARRAY_SIZE()` it's written `; ARRAY_SIZE()`,
>> > which always evaluates to true. The loop then overflows the array and
>> > crashes the kernel.
>>
>> Ooh oops. It was a quick patch I wrote without testing.
>>
>> > By the way, the DualSense code has similar calibration code and also
>> > there the input is not sanitized.
>> > I think it's quite easy to create a fake DualSense device with
>> > an Arduino that emulates the protocol up to calib_denom=0, just to
>> > exploit that and crash every linux machine is plugged in. Or even
>> > worst, exploit that via bluetooth.
>>
>> You are right it probably won't hurt to duplicate the logic there.
>>
>> > >If you want to dig deeper, you can play around with
>> > >dualshock4_get_calibration_data in hid-playstation. The particular
>> > >report (though not fully documented in the driver) does contain a lot
>> > >of device info (firmware info, manufacturing dates, various strings).
>> > >You can probably find the details online. Some data there might be
>> > >weird or not populated.
>> >
>> > Thank you! Just discovered that this strange DS4 (CUH-ZCT2E) replies
>> > all zeros both to HID request 0x02 (get calibration data) and as the
>> > BT address (request 0x12, Pairing Info), which explains why BT is not
>> > working.
>>
>> There is definitely something weird with your device. Something must
>> have gotten wiped somehow.
>>
>> > I tried to request version info (0xa3), the response seems the same as
>> > another fully-working and original CUH-ZCT2E:
>> > """
>> > Build Date: Sep 21 2018 04:50:51
>> > HW Version: 0100.b008
>> > SW Version: 00000001.a00a
>> > SW Series:  2010
>> > Code Size:  0002a000
>> > """
>> >
>> > Anyway, I have seen that request 0xA2 puts the DS4 in an alternate
>> > boot mode, probably DFU mode, where the device reboots as 054c:0856
>> > and waits for data, which seems totally undocumented online.
>> > Do you know anything about this mode?
>> > It would be amazing to be able to reflash the original firmware,
>>
>> Unfortunately I can't disclose any of that information. I can say that
>> on DS4 it wasn't common to update firmware (as opposed to DualSense)
>> while out in the wild. Some of these requests and others are probably
>> used to initiate firmware programming and programming of MAC address,
>> calibration data and other settings. It is probably quite involved and
>> hard to get right without bricking your device.
>>
>> > Thanks,
>> > Alain
>>
>> Thanks,
>> Roderick