Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20191105141807.27054-1-tranmanphong@gmail.com>
In-Reply-To: <20191105141807.27054-1-tranmanphong@gmail.com>
From:   Benjamin Tissoires <benjamin.tissoires@redhat.com>
Date:   Mon, 18 Nov 2019 10:43:21 +0100
Message-ID: <CAO-hwJ+cydMPQE_otc8-67=SDKmjac5RXsLs-9x6dH4YqA+DVQ@mail.gmail.com>
Subject: Re: [PATCH] HID: hid-lg4ff: Fix uninit-value set_autocenter_default
To:     Phong Tran <tranmanphong@gmail.com>
Cc:     "open list:HID CORE LAYER" <linux-input@vger.kernel.org>,
        lkml <linux-kernel@vger.kernel.org>,
        linux-kernel-mentees@lists.linuxfoundation.org,
        syzbot+1234691fec1b8ceba8b1@syzkaller.appspotmail.com
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Nov 5, 2019 at 3:18 PM Phong Tran <tranmanphong@gmail.com> wrote:
>
> syzbot found a problem using of uinit pointer in
> lg4ff_set_autocenter_default().
>
> Reported-by: syzbot+1234691fec1b8ceba8b1@syzkaller.appspotmail.com
>
> Tested by syzbot:
>
> https://groups.google.com/d/msg/syzkaller-bugs/ApnMLW6sfKE/Qq0bIHGEAQAJ

This seems weird to me:

the syzbot link above is about `hid_get_drvdata(hid)`, and, as I read
it, the possibility that hid might not have an initialized value.

Here you are changing the initialized values of value, entry and
drv_data, all 3 are never used before their first assignment.

I have a feeling this particular syzbot check has already been fixed
upstream by d9d4b1e46d95 "HID: Fix assumption that devices have
inputs".

Cheers,
Benjamin

>
> Signed-off-by: Phong Tran <tranmanphong@gmail.com>
> ---
>  drivers/hid/hid-lg4ff.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
> index 5e6a0cef2a06..44dfd08b0c32 100644
> --- a/drivers/hid/hid-lg4ff.c
> +++ b/drivers/hid/hid-lg4ff.c
> @@ -468,10 +468,10 @@ static int lg4ff_play(struct input_dev *dev, void *=
data, struct ff_effect *effec
>  static void lg4ff_set_autocenter_default(struct input_dev *dev, u16 magn=
itude)
>  {
>         struct hid_device *hid =3D input_get_drvdata(dev);
> -       s32 *value;
> +       s32 *value =3D NULL;
>         u32 expand_a, expand_b;
> -       struct lg4ff_device_entry *entry;
> -       struct lg_drv_data *drv_data;
> +       struct lg4ff_device_entry *entry =3D NULL;
> +       struct lg_drv_data *drv_data =3D NULL;
>         unsigned long flags;
>
>         drv_data =3D hid_get_drvdata(hid);
> --
> 2.20.1
>