[<prev] [next>] [day] [month] [year] [list]
Message-ID: <38279d91-402c-7f8b-273d-0882a0567a0c@endrift.com>
Date: Mon, 17 Apr 2023 03:24:52 -0700
From: Vicki Pfau <vi@...rift.com>
To: Dongliang Mu <dzm91@...t.edu.cn>, Dan Carpenter <error27@...il.com>
Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>,
Pavel Rojtberg <rojtberg@...il.com>,
Nate Yocom <nate@...om.org>,
Mattijs Korpershoek <mkorpershoek@...libre.com>,
John Butler <radon86dev@...il.com>,
Matthias Benkmann <matthias.benkmann@...il.com>,
Christopher Crockett <chaorace@...il.com>,
Santosh De Massari <s.demassari@...il.com>,
hust-os-kernel-patches@...glegroups.com,
syzbot+a3f758b8d8cb7e49afec@...kaller.appspotmail.com,
"Pierre-Loup A. Griffais" <pgriffais@...vesoftware.com>,
linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Input: xpad - fix GPF in xpad_probe
Hi,
On 4/17/23 03:01, Dongliang Mu wrote:
>
> On 2023/4/17 17:25, Dan Carpenter wrote:
>> On Fri, Apr 14, 2023 at 08:55:47PM +0800, Dongliang Mu wrote:
>>> In xpad_probe(), it does not allocate xpad->dev with input_dev type.
>>> Then, when it invokes dev_warn with 1st argument - &xpad->dev->dev, it
>>> would trigger GPF.
>> What is a call tree for this? Actually I found it from the bug report.
>> drivers/input/joystick/xpad.c
>> 2034 if (error)
>> 2035 dev_warn(&xpad->dev->dev,
>> 2036 "unable to receive magic message: %d\n",
>> 2037 error);
>> 2038 }
>
Sorry, this appears to be my code, and was merged recently after a few drafts with Dmitry. This code is sensitive to being moved and only affects some controllers, so I'm looking into if I can move it into after creation of the input_dev right now. It's something I'd already thought might be necessary, but I didn't find any evidence for it before. I'll try to get back to you on that soon.
> Hi Dan,
>
> this only occurs in linux-next tree.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/input/joystick/xpad.c?n2053#n2053
>
>>> Fix this by allocating xpad->dev, its error handling and cleanup
>>> operations in the remove function.
>>>
>>> Note that this crash does not have any reproducer, so the patch
>>> only passes compilation testing.
>> The xpad->dev = input_dev; already happens in xpad_init_input(). We
>> shouldn't allocate it twice. I think the fix is to just use a different
>> device pointer for the dev_warn(). Why not use &xpad->intf->dev?
>
> Yeah, the allocation and assignment is in the last part that I missed before. We have two choices to fix this issue:
>
> 1. Change to another device pointer;
>
> 2. Move the allocation and assignment to a previous code site;
>
> If there is no other places dereferencing this pointer before the allocation and assignment, it's better to use the 1st one.
>
> Let me craft a v2 patch.
>
>>
>>> Reported-by: syzbot+a3f758b8d8cb7e49afec@...kaller.appspotmail.com
>> Could you use a Link tag to link to the bug report?
>> Link: https://groups.google.com/g/syzkaller-bugs/c/iMhTgpGuIbM
> Sure, no problem.
>>
>> This needs a Fixes tag.
>>
>> Fixes: db7220c48d8d ("Input: xpad - fix support for some third-party controllers")
>>
>> regards,
>> dan carpenter
>>
Vicki
Powered by blists - more mailing lists