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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Alain,

Just a brief update. I'm still looking at this and I'm not yet sure
what to do. It looks like there is something wrong with the value
ranges of the gyroscope. I need to dig deep in my memory how this code
came around. Some parts of the sensors code were developed and tested
by a team member, who left. For testing he used some internal test 3D
application in which you center some object. There is some validation
there on the values. However, there is just something strange.

The uncalibrated gyroscope values are 16-bit signed, but the hardware
limits are +/- 2000 degree per second. To have enough precision during
calibration the value range is extended (resolution per axis is set to
1024, so 1024 corresponds to 1 degree per second).

When I heavily shake a DS4 in evtest, I can easily reach values like
15 million or more. However, the maximum value we have set on the
evdev node to about 2M. In other words there is something wrong. I
need to look closely at other internal code.

I really hate this as if true, the DS4 unit tests in hid-tools (and
Android) are broken too.

Thanks,
Roderick

On Fri, Dec 30, 2022 at 1:53 PM Roderick Colenbrander
<thunderbird2k@xxxxxxxxx> wrote:
>
> 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@xxxxxxxxx> 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@xxxxxxxxx> ha scritto:
> >>
> >> Hi Alain,
> >>
> >> On Thu, Dec 29, 2022 at 11:21 AM Alain Carlucci
> >> <alain.carlucci@xxxxxxxxx> 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




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux