diff mbox series

drm/vc4: tests: Fix UAF in the mock helpers

Message ID 20231024105640.352752-1-mripard@kernel.org (mailing list archive)
State New, archived
Headers show
Series drm/vc4: tests: Fix UAF in the mock helpers | expand

Commit Message

Maxime Ripard Oct. 24, 2023, 10:56 a.m. UTC
The VC4 mock helpers allocate the CRTC, encoders and connectors using a
call to kunit_kzalloc(), but the DRM device they are attache to survives
for longer than the test itself which leads to use-after-frees reported
by KASAN.

Switch to drmm_kzalloc to tie the lifetime of these objects to the main
DRM device.

Fixes: f759f5b53f1c ("drm/vc4: tests: Introduce a mocking infrastructure")
Closes: https://lore.kernel.org/all/CA+G9fYvJA2HGqzR9LGgq63v0SKaUejHAE6f7+z9cwWN-ourJ_g@mail.gmail.com/
Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
Signed-off-by: Maxime Ripard <mripard@kernel.org>

---

Cc: Naresh Kamboju <naresh.kamboju@linaro.org>
Cc: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c   | 2 +-
 drivers/gpu/drm/vc4/tests/vc4_mock_output.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Maíra Canal Oct. 24, 2023, 4:38 p.m. UTC | #1
On 10/24/23 07:56, Maxime Ripard wrote:
> The VC4 mock helpers allocate the CRTC, encoders and connectors using a
> call to kunit_kzalloc(), but the DRM device they are attache to survives
> for longer than the test itself which leads to use-after-frees reported
> by KASAN.
> 
> Switch to drmm_kzalloc to tie the lifetime of these objects to the main
> DRM device.
> 
> Fixes: f759f5b53f1c ("drm/vc4: tests: Introduce a mocking infrastructure")
> Closes: https://lore.kernel.org/all/CA+G9fYvJA2HGqzR9LGgq63v0SKaUejHAE6f7+z9cwWN-ourJ_g@mail.gmail.com/
> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>

Reviewed-by: Maíra Canal <mcanal@igalia.com>

Best Regards,
- Maíra Canal

> 
> ---
> 
> Cc: Naresh Kamboju <naresh.kamboju@linaro.org>
> Cc: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>   drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c   | 2 +-
>   drivers/gpu/drm/vc4/tests/vc4_mock_output.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c b/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c
> index 5d12d7beef0e..ade3309ae042 100644
> --- a/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c
> +++ b/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c
> @@ -26,7 +26,7 @@ struct vc4_dummy_crtc *vc4_mock_pv(struct kunit *test,
>   	struct vc4_crtc *vc4_crtc;
>   	int ret;
>   
> -	dummy_crtc = kunit_kzalloc(test, sizeof(*dummy_crtc), GFP_KERNEL);
> +	dummy_crtc = drmm_kzalloc(drm, sizeof(*dummy_crtc), GFP_KERNEL);
>   	KUNIT_ASSERT_NOT_NULL(test, dummy_crtc);
>   
>   	vc4_crtc = &dummy_crtc->crtc;
> diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock_output.c b/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
> index 6e11fcc9ef45..e70d7c3076ac 100644
> --- a/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
> +++ b/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
> @@ -32,7 +32,7 @@ struct vc4_dummy_output *vc4_dummy_output(struct kunit *test,
>   	struct drm_encoder *enc;
>   	int ret;
>   
> -	dummy_output = kunit_kzalloc(test, sizeof(*dummy_output), GFP_KERNEL);
> +	dummy_output = drmm_kzalloc(drm, sizeof(*dummy_output), GFP_KERNEL);
>   	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dummy_output);
>   	dummy_output->encoder.type = vc4_encoder_type;
>
Anders Roxell Oct. 26, 2023, 10:58 a.m. UTC | #2
On Tue, 24 Oct 2023 at 18:38, Maíra Canal <mcanal@igalia.com> wrote:
>
> On 10/24/23 07:56, Maxime Ripard wrote:
> > The VC4 mock helpers allocate the CRTC, encoders and connectors using a
> > call to kunit_kzalloc(), but the DRM device they are attache to survives
> > for longer than the test itself which leads to use-after-frees reported
> > by KASAN.
> >
> > Switch to drmm_kzalloc to tie the lifetime of these objects to the main
> > DRM device.
> >
> > Fixes: f759f5b53f1c ("drm/vc4: tests: Introduce a mocking infrastructure")
> > Closes: https://lore.kernel.org/all/CA+G9fYvJA2HGqzR9LGgq63v0SKaUejHAE6f7+z9cwWN-ourJ_g@mail.gmail.com/
> > Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
>
> Reviewed-by: Maíra Canal <mcanal@igalia.com>

Tested-by: Anders Roxell <anders.roxell@linaro.org>

I tested this patch ontop of next-20231025 and on next-20231011 when
we first saw the issue,
both kernels booted fine on a RPI4(bcm2711-rpi-4-b)

Cheers,
Anders


>
> Best Regards,
> - Maíra Canal
>
> >
> > ---
> >
> > Cc: Naresh Kamboju <naresh.kamboju@linaro.org>
> > Cc: Dan Carpenter <dan.carpenter@linaro.org>
> > ---
> >   drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c   | 2 +-
> >   drivers/gpu/drm/vc4/tests/vc4_mock_output.c | 2 +-
> >   2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c b/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c
> > index 5d12d7beef0e..ade3309ae042 100644
> > --- a/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c
> > +++ b/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c
> > @@ -26,7 +26,7 @@ struct vc4_dummy_crtc *vc4_mock_pv(struct kunit *test,
> >       struct vc4_crtc *vc4_crtc;
> >       int ret;
> >
> > -     dummy_crtc = kunit_kzalloc(test, sizeof(*dummy_crtc), GFP_KERNEL);
> > +     dummy_crtc = drmm_kzalloc(drm, sizeof(*dummy_crtc), GFP_KERNEL);
> >       KUNIT_ASSERT_NOT_NULL(test, dummy_crtc);
> >
> >       vc4_crtc = &dummy_crtc->crtc;
> > diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock_output.c b/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
> > index 6e11fcc9ef45..e70d7c3076ac 100644
> > --- a/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
> > +++ b/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
> > @@ -32,7 +32,7 @@ struct vc4_dummy_output *vc4_dummy_output(struct kunit *test,
> >       struct drm_encoder *enc;
> >       int ret;
> >
> > -     dummy_output = kunit_kzalloc(test, sizeof(*dummy_output), GFP_KERNEL);
> > +     dummy_output = drmm_kzalloc(drm, sizeof(*dummy_output), GFP_KERNEL);
> >       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dummy_output);
> >       dummy_output->encoder.type = vc4_encoder_type;
> >
Maxime Ripard Oct. 26, 2023, 11:05 a.m. UTC | #3
On Tue, 24 Oct 2023 12:56:40 +0200, Maxime Ripard wrote:
> The VC4 mock helpers allocate the CRTC, encoders and connectors using a
> call to kunit_kzalloc(), but the DRM device they are attache to survives
> for longer than the test itself which leads to use-after-frees reported
> by KASAN.
> 
> Switch to drmm_kzalloc to tie the lifetime of these objects to the main
> DRM device.
> 
> [...]

Applied to drm/drm-misc (drm-misc-fixes).

Thanks!
Maxime
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c b/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c
index 5d12d7beef0e..ade3309ae042 100644
--- a/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c
+++ b/drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c
@@ -26,7 +26,7 @@  struct vc4_dummy_crtc *vc4_mock_pv(struct kunit *test,
 	struct vc4_crtc *vc4_crtc;
 	int ret;
 
-	dummy_crtc = kunit_kzalloc(test, sizeof(*dummy_crtc), GFP_KERNEL);
+	dummy_crtc = drmm_kzalloc(drm, sizeof(*dummy_crtc), GFP_KERNEL);
 	KUNIT_ASSERT_NOT_NULL(test, dummy_crtc);
 
 	vc4_crtc = &dummy_crtc->crtc;
diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock_output.c b/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
index 6e11fcc9ef45..e70d7c3076ac 100644
--- a/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
+++ b/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
@@ -32,7 +32,7 @@  struct vc4_dummy_output *vc4_dummy_output(struct kunit *test,
 	struct drm_encoder *enc;
 	int ret;
 
-	dummy_output = kunit_kzalloc(test, sizeof(*dummy_output), GFP_KERNEL);
+	dummy_output = drmm_kzalloc(drm, sizeof(*dummy_output), GFP_KERNEL);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dummy_output);
 	dummy_output->encoder.type = vc4_encoder_type;