[PATCH] HID: hid-lg4ff: Fix uninit-value set_autocenter_default

Subsystems: hid core layer, the rest

STALE2442d

4 messages, 2 authors, 2019-11-25 · open the first message on its own page

[PATCH] HID: hid-lg4ff: Fix uninit-value set_autocenter_default

From: Phong Tran <hidden>
Date: 2019-11-05 14:18:24 

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

Signed-off-by: Phong Tran <redacted>
---
 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 magnitude)
 {
 	struct hid_device *hid = input_get_drvdata(dev);
-	s32 *value;
+	s32 *value = NULL;
 	u32 expand_a, expand_b;
-	struct lg4ff_device_entry *entry;
-	struct lg_drv_data *drv_data;
+	struct lg4ff_device_entry *entry = NULL;
+	struct lg_drv_data *drv_data = NULL;
 	unsigned long flags;
 
 	drv_data = hid_get_drvdata(hid);
-- 
2.20.1

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH] HID: hid-lg4ff: Fix uninit-value set_autocenter_default

From: Benjamin Tissoires <hidden>
Date: 2019-11-18 09:43:41 

On Tue, Nov 5, 2019 at 3:18 PM Phong Tran [off-list ref] 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
quoted hunk
Signed-off-by: Phong Tran <redacted>
---
 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 magnitude)
 {
        struct hid_device *hid = input_get_drvdata(dev);
-       s32 *value;
+       s32 *value = NULL;
        u32 expand_a, expand_b;
-       struct lg4ff_device_entry *entry;
-       struct lg_drv_data *drv_data;
+       struct lg4ff_device_entry *entry = NULL;
+       struct lg_drv_data *drv_data = NULL;
        unsigned long flags;

        drv_data = hid_get_drvdata(hid);
--
2.20.1
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH] HID: hid-lg4ff: Fix uninit-value set_autocenter_default

From: Phong Tran <hidden>
Date: 2019-11-19 13:30:01 

On 11/18/19 4:43 PM, Benjamin Tissoires wrote:
On Tue, Nov 5, 2019 at 3:18 PM Phong Tran [off-list ref] wrote:
quoted
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.
In the dashboard [1] shows
BUG: KMSAN: uninit-value in dev_get_drvdata include/linux/device.h:1388 
[inline]
BUG: KMSAN: uninit-value in hid_get_drvdata include/linux/hid.h:628 [inline]
BUG: KMSAN: uninit-value in lg4ff_set_autocenter_default+0x23a/0xa20 
drivers/hid/hid-lg4ff.c:477
base on that I did the initialization the pointer in the patch.

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".
I think the commit d9d4b1 fixed this report [2] by syzbot.

[1] https://syzkaller.appspot.com/bug?extid=1234691fec1b8ceba8b1
[2] https://syzkaller.appspot.com/bug?extid=403741a091bf41d4ae79

regards,
Phong.
Cheers,
Benjamin
quoted
Signed-off-by: Phong Tran <redacted>
---
  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 magnitude)
  {
         struct hid_device *hid = input_get_drvdata(dev);
-       s32 *value;
+       s32 *value = NULL;
         u32 expand_a, expand_b;
-       struct lg4ff_device_entry *entry;
-       struct lg_drv_data *drv_data;
+       struct lg4ff_device_entry *entry = NULL;
+       struct lg_drv_data *drv_data = NULL;
         unsigned long flags;

         drv_data = hid_get_drvdata(hid);
--
2.20.1
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH] HID: hid-lg4ff: Fix uninit-value set_autocenter_default

From: Benjamin Tissoires <hidden>
Date: 2019-11-25 15:14:50 

On Tue, Nov 19, 2019 at 2:30 PM Phong Tran [off-list ref] wrote:
On 11/18/19 4:43 PM, Benjamin Tissoires wrote:
quoted
On Tue, Nov 5, 2019 at 3:18 PM Phong Tran [off-list ref] wrote:
quoted
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.
In the dashboard [1] shows
BUG: KMSAN: uninit-value in dev_get_drvdata include/linux/device.h:1388
[inline]
BUG: KMSAN: uninit-value in hid_get_drvdata include/linux/hid.h:628 [inline]
BUG: KMSAN: uninit-value in lg4ff_set_autocenter_default+0x23a/0xa20
drivers/hid/hid-lg4ff.c:477
base on that I did the initialization the pointer in the patch.
quoted
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".
I think the commit d9d4b1 fixed this report [2] by syzbot.

[1] https://syzkaller.appspot.com/bug?extid=1234691fec1b8ceba8b1
[2] https://syzkaller.appspot.com/bug?extid=403741a091bf41d4ae79
As you can see in my long discussion with syzbot today, d9d4b1 also
fixed that one.

https://groups.google.com/forum/#!msg/syzkaller-bugs/ApnMLW6sfKE/Qq0bIHGEAQAJ

Cheers,
Benjamin

regards,
Phong.
quoted
Cheers,
Benjamin
quoted
Signed-off-by: Phong Tran <redacted>
---
  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 magnitude)
  {
         struct hid_device *hid = input_get_drvdata(dev);
-       s32 *value;
+       s32 *value = NULL;
         u32 expand_a, expand_b;
-       struct lg4ff_device_entry *entry;
-       struct lg_drv_data *drv_data;
+       struct lg4ff_device_entry *entry = NULL;
+       struct lg_drv_data *drv_data = NULL;
         unsigned long flags;

         drv_data = hid_get_drvdata(hid);
--
2.20.1
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help