[PATCH] drivers/hid/hid-multitouch.c: fix a possible null pointer access.

[PATCH] drivers/hid/hid-multitouch.c: fix a possible null pointer access.

[z00417012]

1002     if ((quirks & MT_QUIRK_IGNORE_DUPLICATES) && mt) {
1003         struct input_mt_slot *i_slot = &mt->slots[slotnum];
1004 
1005         if (input_mt_is_active(i_slot) &&
1006             input_mt_is_used(mt, i_slot))
1007             return -EAGAIN;
1008     }

We previously assumed 'mt' could be null (see line 1002).

The following situation is similar, so add a judgement.

Signed-off-by: Pan Zhang <zhangpan26@huawei.com>
---
 drivers/hid/hid-multitouch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 3cfeb16..368de81 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1019,7 +1019,7 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
 		tool = MT_TOOL_DIAL;
 	else if (unlikely(!confidence_state)) {
 		tool = MT_TOOL_PALM;
-		if (!active &&
+		if (!active && mt
 		    input_mt_is_active(&mt->slots[slotnum])) {
 			/*
 			 * The non-confidence was reported for
-- 
2.7.4

Re: [PATCH] drivers/hid/hid-multitouch.c: fix a possible null pointer access.

[Benjamin Tissoires]

Hi,

On Tue, Dec 17, 2019 at 4:17 AM z00417012 <zhangpan26@huawei.com> wrote:

Can you add at the beginning of your commit message:
From: Pan Zhang <zhangpan26@huawei.com>

This way we have the commit author that matches the signature, which
is a requirement for the kernel.

>
> 1002     if ((quirks & MT_QUIRK_IGNORE_DUPLICATES) && mt) {
> 1003         struct input_mt_slot *i_slot = &mt->slots[slotnum];
> 1004
> 1005         if (input_mt_is_active(i_slot) &&
> 1006             input_mt_is_used(mt, i_slot))
> 1007             return -EAGAIN;
> 1008     }
>
> We previously assumed 'mt' could be null (see line 1002).
>
> The following situation is similar, so add a judgement.
>
> Signed-off-by: Pan Zhang <zhangpan26@huawei.com>
> ---
>  drivers/hid/hid-multitouch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 3cfeb16..368de81 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -1019,7 +1019,7 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
>                 tool = MT_TOOL_DIAL;
>         else if (unlikely(!confidence_state)) {
>                 tool = MT_TOOL_PALM;
> -               if (!active &&
> +               if (!active && mt

Ack on the principle, but this doesn't even compile. You are missing a
`&&` at the end of the line.

Can you send a v2 with the comments above? And we will queue the v2
for 5.5 I think.

Cheers,
Benjamin

>                     input_mt_is_active(&mt->slots[slotnum])) {
>                         /*
>                          * The non-confidence was reported for
> --
> 2.7.4
>

Re: Re: [PATCH v2] drivers/hid/hid-multitouch.c: fix a possible null pointer access.

[Pan Zhang]

On Tue, Dec 17, 2019 at 18:50 PM Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:

>Can you add at the beginning of your commit message:
>From: Pan Zhang <zhangpan26@huawei.com>
>
>This way we have the commit author that matches the signature, which is a requirement for the kernel.

Firstly, thanks for your reviewing very much. I would fix my signature.

>>  drivers/hid/hid-multitouch.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/hid-multitouch.c 
>> b/drivers/hid/hid-multitouch.c index 3cfeb16..368de81 100644
>> --- a/drivers/hid/hid-multitouch.c
>> +++ b/drivers/hid/hid-multitouch.c
>> @@ -1019,7 +1019,7 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
>>                 tool = MT_TOOL_DIAL;
>>         else if (unlikely(!confidence_state)) {
>>                 tool = MT_TOOL_PALM;
>> -               if (!active &&
>> +               if (!active && mt

>Ack on the principle, but this doesn't even compile. You are missing a `&&` at the end of the line.
>
>Can you send a v2 with the comments above? And we will queue the v2 for 5.5 I think.

Sorry about that. I made a stupid mistake. This patch fixed it.

Signed-off-by: Pan Zhang <zhangpan26@huawei.com>
---
 drivers/hid/hid-multitouch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 3cfeb16..368de81 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1019,7 +1019,7 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
 		tool = MT_TOOL_DIAL;
 	else if (unlikely(!confidence_state)) {
 		tool = MT_TOOL_PALM;
-		if (!active &&
+		if (!active && mt &&
 		    input_mt_is_active(&mt->slots[slotnum])) {
 			/*
 			 * The non-confidence was reported for
-- 
2.7.4

Re: Re: [PATCH v2] drivers/hid/hid-multitouch.c: fix a possible null pointer access.

[Benjamin Tissoires]

Hi,

On Tue, Dec 17, 2019 at 1:28 PM Pan Zhang <zhangpan26@huawei.com> wrote:
>
> On Tue, Dec 17, 2019 at 18:50 PM Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
>
> >Can you add at the beginning of your commit message:
> >From: Pan Zhang <zhangpan26@huawei.com>
> >
> >This way we have the commit author that matches the signature, which is a requirement for the kernel.
>
> Firstly, thanks for your reviewing very much. I would fix my signature.
>
> >>  drivers/hid/hid-multitouch.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/hid/hid-multitouch.c
> >> b/drivers/hid/hid-multitouch.c index 3cfeb16..368de81 100644
> >> --- a/drivers/hid/hid-multitouch.c
> >> +++ b/drivers/hid/hid-multitouch.c
> >> @@ -1019,7 +1019,7 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
> >>                 tool = MT_TOOL_DIAL;
> >>         else if (unlikely(!confidence_state)) {
> >>                 tool = MT_TOOL_PALM;
> >> -               if (!active &&
> >> +               if (!active && mt
>
> >Ack on the principle, but this doesn't even compile. You are missing a `&&` at the end of the line.
> >
> >Can you send a v2 with the comments above? And we will queue the v2 for 5.5 I think.
>
> Sorry about that. I made a stupid mistake. This patch fixed it.

No worries, mistakes happen.

However, can you resend the patch to the LKML in a separate thread?
Also prefix the patch with "[PATCH v2]". It's easier for our tools to
handle patches when they are send on their own. Because here, I would
have to manually edit either the commit message removing the thread,
either take the first version and edit the patch. It's not so hard for
this kind of simple patches, but it's best to take good habits :)

Cheers,
Benjamin

>
> Signed-off-by: Pan Zhang <zhangpan26@huawei.com>
> ---
>  drivers/hid/hid-multitouch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 3cfeb16..368de81 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -1019,7 +1019,7 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
>                 tool = MT_TOOL_DIAL;
>         else if (unlikely(!confidence_state)) {
>                 tool = MT_TOOL_PALM;
> -               if (!active &&
> +               if (!active && mt &&
>                     input_mt_is_active(&mt->slots[slotnum])) {
>                         /*
>                          * The non-confidence was reported for
> --
> 2.7.4
>