Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210411150623.8367-1-hcvcastro@gmail.com> <CAEc3jaAADGuPDwGLM6HskLq-OiecbV-76JbMe6yambVAT=h=bw@mail.gmail.com>
 <CAF44FUyskn-g+MUYONMXYZ8dUiGFd0GF1wkkPQwd34ikbet_Fg@mail.gmail.com>
In-Reply-To: <CAF44FUyskn-g+MUYONMXYZ8dUiGFd0GF1wkkPQwd34ikbet_Fg@mail.gmail.com>
From:   Roderick Colenbrander <thunderbird2k@gmail.com>
Date:   Mon, 26 Apr 2021 09:36:56 -0700
Message-ID: <CAEc3jaB+wNp1Fa-_P=XcYxRNL+L4z_M_wsW6iODyyHz90Xq-vw@mail.gmail.com>
Subject: Re: [PATCH] drivers/hid: avoid invalid denominator
To:     Henry Castro <hcvcastro@gmail.com>
Cc:     Jiri Kosina <jikos@kernel.org>,
        Benjamin Tissoires <benjamin.tissoires@redhat.com>,
        linux-input <linux-input@vger.kernel.org>,
        lkml <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hm, I'm a little confused then. The Shanwan gamepad is a Dualshock 3
clone (nickname 'sixaxis'), so should not go through the DualShock 4
paths. Can you double check what the vendor / device id for this
device are? Are you seeing specific errors or warnings from the driver
in the kernel logs?

I have been considering how to better handle some of these clones
devices, while also not polluting the code too much. If this device is
indeed a clone DualShock 4. I am more leaning towards finding out
sooner during device creation. Perhaps should not even create a motion
sensor device if the data will be none anyway... Hm, not sure what I
prefer yet.

Thanks,
Roderick

On Sun, Apr 25, 2021 at 9:39 AM Henry Castro <hcvcastro@gmail.com> wrote:
>
> Hi,
>
> I was trying to play some old games from Retroarch, and I plugged
> my "SHANWAN PS3 GamePad", but it begin to vibrate and I could
> not play the games, So I started to investigate and read the source
> code, and get the code to bypass the check, and suddenly I got
> a panic crash.  So I found the issue, and I added the patch, finally
> got working, and I played my favorite old games.
>
> So it happens in PS3 clone device I think, but I have to tweak the code
> probably is safe under the condition you mention in PS4 DualShock,
> but it is good as a prevention, if someone is testing and debugging
> the driver.
>
> Regards
> Henry
>
> El s=C3=A1b, 24 abr 2021 a las 17:11, Roderick Colenbrander (<thunderbird=
2k@gmail.com>) escribi=C3=B3:
>>
>> Hi Henry,
>>
>> Thanks for your patch. In what case has this been an issue? Or was it
>> more theoretical.
>>
>> During normal operation this condition should never be triggered for a
>> DualShock 4 when calibration succeeds. If it doesn't succeed the
>> device is not registered. We had an issue recently with the DS4 dongle
>> where the calibration data was 0, which was due to a race condition
>> with Steam, but that was resolved recently.
>>
>> Thanks,
>> Roderick
>>
>> On Sun, Apr 11, 2021 at 10:19 AM Henry Castro <hcvcastro@gmail.com> wrot=
e:
>> >
>> > Avoid a potential panic in case wrong denominator
>> > is given.
>> >
>> > Signed-off-by: Henry Castro <hcvcastro@gmail.com>
>> > ---
>> >  drivers/hid/hid-sony.c | 13 +++++++++----
>> >  1 file changed, 9 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
>> > index 8319b0ce385a..67b45d82cc3b 100644
>> > --- a/drivers/hid/hid-sony.c
>> > +++ b/drivers/hid/hid-sony.c
>> > @@ -1134,11 +1134,16 @@ static void dualshock4_parse_report(struct son=
y_sc *sc, u8 *rd, int size)
>> >                  * Note: we swap numerator 'x' and 'numer' in mult_fra=
c for
>> >                  *       precision reasons so we don't need 64-bit.
>> >                  */
>> > -               int calib_data =3D mult_frac(calib->sens_numer,
>> > -                                          raw_data - calib->bias,
>> > -                                          calib->sens_denom);
>> > +               if (calib->sens_denom !=3D 0) {
>> > +                       int calib_data =3D mult_frac(calib->sens_numer=
,
>> > +                                                  raw_data - calib->b=
ias,
>> > +                                                  calib->sens_denom);
>> > +
>> > +                       input_report_abs(sc->sensor_dev, calib->abs_co=
de, calib_data);
>> > +               } else {
>> > +                       hid_warn(sc->hdev, "DualShock 4 parse report, =
avoid invalid denominator");
>> > +               }
>> >
>> > -               input_report_abs(sc->sensor_dev, calib->abs_code, cali=
b_data);
>> >                 offset +=3D 2;
>> >         }
>> >         input_sync(sc->sensor_dev);
>> > --
>> > 2.20.1
>> >