| Message ID | cover.1683022164.git.geert+renesas@glider.be |
|---|---|
| Headers | show |
| Series | Input: tests - miscellaneous fixes | expand |
Geert Uytterhoeven <geert@linux-m68k.org> writes: Hello Geert, > Hi Javier, > > On Tue, May 2, 2023 at 12:17 PM Geert Uytterhoeven > <geert+renesas@glider.be> wrote: >> This patch series fixes a crash in the new input selftest, and makes the >> test available when the KUnit framework is modular. >> >> Unfortunately test 3 still fails for me (tested on Koelsch (R-Car M2-W) >> and ARAnyM): >> >> KTAP version 1 >> # Subtest: input_core >> 1..3 >> input: Test input device as /devices/virtual/input/input1 >> ok 1 input_test_polling >> input: Test input device as /devices/virtual/input/input2 >> ok 2 input_test_timestamp >> input: Test input device as /devices/virtual/input/input3 >> # input_test_match_device_id: ASSERTION FAILED at # drivers/input/tests/input_test.c:99 >> Expected input_match_device_id(input_dev, &id) to be true, but is false >> not ok 3 input_test_match_device_id >> # input_core: pass:2 fail:1 skip:0 total:3 >> # Totals: pass:2 fail:1 skip:0 total:3 >> not ok 1 input_core > > Adding more debug code shows that it's the test on evbit [1] in > input_match_device_id() that fails. > Looking at your input_test_match_device_id(), I think you expect > the checks for the various bitmaps to be gated by > "if (id->flags & INPUT_DEVICE_ID_MATCH_EVBIT)", like is done for the > other checks? > > [1] https://elixir.bootlin.com/linux/latest/source/drivers/input/input.c#L1021 > That's correct. In input_test_init(), the input dev is marked as capable of emitting EV_KEY BTN_LEFT and BTN_RIGHT. The goal of that test was to check this. That is, check if matches by the input dev capabilities in which case the __set_bit(EV_KEY, ...) would make the match true and __set_bit(EV_ABS, ..) would make the condition false. But maybe I misunderstood how the input_set_capability() and __set_bit() functions work ? I'll take a look to this tomorrow, thanks a lot for your report!
Hi Dmitry, On Tue, May 2, 2023 at 7:05 PM Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > On Tue, May 02, 2023 at 06:31:51PM +0200, Javier Martinez Canillas wrote: > > Geert Uytterhoeven <geert@linux-m68k.org> writes: > > > On Tue, May 2, 2023 at 12:17 PM Geert Uytterhoeven > > > <geert+renesas@glider.be> wrote: > > >> This patch series fixes a crash in the new input selftest, and makes the > > >> test available when the KUnit framework is modular. > > >> > > >> Unfortunately test 3 still fails for me (tested on Koelsch (R-Car M2-W) > > >> and ARAnyM): > > >> > > >> KTAP version 1 > > >> # Subtest: input_core > > >> 1..3 > > >> input: Test input device as /devices/virtual/input/input1 > > >> ok 1 input_test_polling > > >> input: Test input device as /devices/virtual/input/input2 > > >> ok 2 input_test_timestamp > > >> input: Test input device as /devices/virtual/input/input3 > > >> # input_test_match_device_id: ASSERTION FAILED at # drivers/input/tests/input_test.c:99 > > >> Expected input_match_device_id(input_dev, &id) to be true, but is false > > >> not ok 3 input_test_match_device_id > > >> # input_core: pass:2 fail:1 skip:0 total:3 > > >> # Totals: pass:2 fail:1 skip:0 total:3 > > >> not ok 1 input_core > > > > > > Adding more debug code shows that it's the test on evbit [1] in > > > input_match_device_id() that fails. > > > Looking at your input_test_match_device_id(), I think you expect > > > the checks for the various bitmaps to be gated by > > > "if (id->flags & INPUT_DEVICE_ID_MATCH_EVBIT)", like is done for the > > > other checks? > > > > > > [1] https://elixir.bootlin.com/linux/latest/source/drivers/input/input.c#L1021 > > > > > > > That's correct. In input_test_init(), the input dev is marked as capable > > of emitting EV_KEY BTN_LEFT and BTN_RIGHT. The goal of that test was to > > check this. > > > > That is, check if matches by the input dev capabilities in which case the > > __set_bit(EV_KEY, ...) would make the match true and __set_bit(EV_ABS, ..) > > would make the condition false. > > > > But maybe I misunderstood how the input_set_capability() and __set_bit() > > functions work ? > > > > I'll take a look to this tomorrow, thanks a lot for your report! > > Unfortunately (?) INPUT_DEVICE_ID_MATCH_*BIT have never had any effect, > the kernel always used bitmaps when matching (with the assumption that > if one does not care about given bitmap they can simply pass empty one), > so I think what we need to change is: > > diff --git a/drivers/input/tests/input_test.c b/drivers/input/tests/input_test.c > index 8b8ac3412a70..0540225f0288 100644 > --- a/drivers/input/tests/input_test.c > +++ b/drivers/input/tests/input_test.c > @@ -87,7 +87,7 @@ static void input_test_timestamp(struct kunit *test) > static void input_test_match_device_id(struct kunit *test) > { > struct input_dev *input_dev = test->priv; > - struct input_device_id id; > + struct input_device_id id = { 0 }; > > /* > * Must match when the input device bus, vendor, product, version > > to avoid having garbage in the match data. Thanks, that did the trick! 3/3 tests pass. Gr{oetje,eeting}s, Geert
Geert Uytterhoeven <geert@linux-m68k.org> writes: > Hi Dmitry, > > On Tue, May 2, 2023 at 7:05 PM Dmitry Torokhov > <dmitry.torokhov@gmail.com> wrote: >> On Tue, May 02, 2023 at 06:31:51PM +0200, Javier Martinez Canillas wrote: >> > Geert Uytterhoeven <geert@linux-m68k.org> writes: >> > > On Tue, May 2, 2023 at 12:17 PM Geert Uytterhoeven >> > > <geert+renesas@glider.be> wrote: >> > >> This patch series fixes a crash in the new input selftest, and makes the >> > >> test available when the KUnit framework is modular. >> > >> >> > >> Unfortunately test 3 still fails for me (tested on Koelsch (R-Car M2-W) >> > >> and ARAnyM): >> > >> >> > >> KTAP version 1 >> > >> # Subtest: input_core >> > >> 1..3 >> > >> input: Test input device as /devices/virtual/input/input1 >> > >> ok 1 input_test_polling >> > >> input: Test input device as /devices/virtual/input/input2 >> > >> ok 2 input_test_timestamp >> > >> input: Test input device as /devices/virtual/input/input3 >> > >> # input_test_match_device_id: ASSERTION FAILED at # drivers/input/tests/input_test.c:99 >> > >> Expected input_match_device_id(input_dev, &id) to be true, but is false >> > >> not ok 3 input_test_match_device_id >> > >> # input_core: pass:2 fail:1 skip:0 total:3 >> > >> # Totals: pass:2 fail:1 skip:0 total:3 >> > >> not ok 1 input_core >> > > >> > > Adding more debug code shows that it's the test on evbit [1] in >> > > input_match_device_id() that fails. >> > > Looking at your input_test_match_device_id(), I think you expect >> > > the checks for the various bitmaps to be gated by >> > > "if (id->flags & INPUT_DEVICE_ID_MATCH_EVBIT)", like is done for the >> > > other checks? >> > > >> > > [1] https://elixir.bootlin.com/linux/latest/source/drivers/input/input.c#L1021 >> > > >> > >> > That's correct. In input_test_init(), the input dev is marked as capable >> > of emitting EV_KEY BTN_LEFT and BTN_RIGHT. The goal of that test was to >> > check this. >> > >> > That is, check if matches by the input dev capabilities in which case the >> > __set_bit(EV_KEY, ...) would make the match true and __set_bit(EV_ABS, ..) >> > would make the condition false. >> > >> > But maybe I misunderstood how the input_set_capability() and __set_bit() >> > functions work ? >> > >> > I'll take a look to this tomorrow, thanks a lot for your report! >> >> Unfortunately (?) INPUT_DEVICE_ID_MATCH_*BIT have never had any effect, >> the kernel always used bitmaps when matching (with the assumption that >> if one does not care about given bitmap they can simply pass empty one), >> so I think what we need to change is: >> >> diff --git a/drivers/input/tests/input_test.c b/drivers/input/tests/input_test.c >> index 8b8ac3412a70..0540225f0288 100644 >> --- a/drivers/input/tests/input_test.c >> +++ b/drivers/input/tests/input_test.c >> @@ -87,7 +87,7 @@ static void input_test_timestamp(struct kunit *test) >> static void input_test_match_device_id(struct kunit *test) >> { >> struct input_dev *input_dev = test->priv; >> - struct input_device_id id; >> + struct input_device_id id = { 0 }; >> >> /* >> * Must match when the input device bus, vendor, product, version >> >> to avoid having garbage in the match data. > > Thanks, that did the trick! 3/3 tests pass. > Oh, silly me. Are you going to post that as a proper patch ?
Hi all, This patch series fixes a crash in the new input selftest, and makes the test available when the KUnit framework is modular. Unfortunately test 3 still fails for me (tested on Koelsch (R-Car M2-W) and ARAnyM): KTAP version 1 # Subtest: input_core 1..3 input: Test input device as /devices/virtual/input/input1 ok 1 input_test_polling input: Test input device as /devices/virtual/input/input2 ok 2 input_test_timestamp input: Test input device as /devices/virtual/input/input3 # input_test_match_device_id: ASSERTION FAILED at # drivers/input/tests/input_test.c:99 Expected input_match_device_id(input_dev, &id) to be true, but is false not ok 3 input_test_match_device_id # input_core: pass:2 fail:1 skip:0 total:3 # Totals: pass:2 fail:1 skip:0 total:3 not ok 1 input_core Thanks! Geert Uytterhoeven (2): Input: tests - fix use-after-free and refcount underflow in input_test_exit() Input: tests - modular KUnit tests should not depend on KUNIT=y drivers/input/Kconfig | 2 +- drivers/input/tests/input_test.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)