| Message ID | 1576551722-16966-1-git-send-email-zhangpan26@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | drivers/hid/hid-multitouch.c: fix a possible null pointer access. | expand |
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 >
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
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 >
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
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(-)