linux-kernel - Re: [PATCH 1/2] drm: Add might_fault to drm_modeset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKMK7uFvCr2qcHun06LC-ON3GBqj8=mCpPGHuAOh9BEyr60fiQ@mail.gmail.com>
Date: Wed, 10 Jul 2024 13:58:48 +0200
From: Daniel Vetter <daniel.vetter@...ll.ch>
To: Christian König <christian.koenig@....com>
Cc: DRI Development <dri-devel@...ts.freedesktop.org>, LKML <linux-kernel@...r.kernel.org>, 
	Intel Graphics Development <intel-gfx@...ts.freedesktop.org>, Daniel Vetter <daniel.vetter@...el.com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, Sumit Semwal <sumit.semwal@...aro.org>, 
	linux-media@...r.kernel.org, linaro-mm-sig@...ts.linaro.org
Subject: Re: [PATCH 1/2] drm: Add might_fault to drm_modeset_lock priming

On Wed, 10 Jul 2024 at 13:39, Christian König <christian.koenig@....com> wrote:
>
> Am 10.07.24 um 11:31 schrieb Daniel Vetter:
> > We already teach lockdep that dma_resv nests within drm_modeset_lock,
> > but there's a lot more: All drm kms ioctl rely on being able to
> > put/get_user while holding modeset locks, so we really need a
> > might_fault in there too to complete the picture. Add it.
>
> Mhm, lockdep should be able to deduce that when there might be faults
> under the dma_resv lock there might also be faults under the
> drm_modeset_lock.

You're not allowed to take a fault under dma_resv, because drivers
might need to take that lock to handle faults. So unfortunately in our
combined lockdep priming, there really seems to be no chain yet that
teaches about faults possibly happening while holding
drm_modeset_lock.
-Sima

>
> >
> > Motivated by a syzbot report that blew up on bcachefs doing an
> > unconditional console_lock way deep in the locking hierarchy, and
> > lockdep only noticing the depency loop in a drm ioctl instead of much
> > earlier. This annotation will make sure such issues have a much harder
> > time escaping.
> >
> > References: https://lore.kernel.org/dri-devel/00000000000073db8b061cd43496@google.com/
> > Signed-off-by: Daniel Vetter <daniel.vetter@...el.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>
> > Cc: Maxime Ripard <mripard@...nel.org>
> > Cc: Thomas Zimmermann <tzimmermann@...e.de>
> > Cc: Sumit Semwal <sumit.semwal@...aro.org>
> > Cc: "Christian König" <christian.koenig@....com>
> > Cc: linux-media@...r.kernel.org
> > Cc: linaro-mm-sig@...ts.linaro.org
>
> On the other hand pointing it out explicitly doesn't hurts us at all, so
> Reviewed-by: Christian König <christian.koenig@....com>.
>
> Regards,
> Christian.
>
> > ---
> >   drivers/gpu/drm/drm_mode_config.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> > index 568972258222..37d2e0a4ef4b 100644
> > --- a/drivers/gpu/drm/drm_mode_config.c
> > +++ b/drivers/gpu/drm/drm_mode_config.c
> > @@ -456,6 +456,8 @@ int drmm_mode_config_init(struct drm_device *dev)
> >               if (ret == -EDEADLK)
> >                       ret = drm_modeset_backoff(&modeset_ctx);
> >
> > +             might_fault();
> > +
> >               ww_acquire_init(&resv_ctx, &reservation_ww_class);
> >               ret = dma_resv_lock(&resv, &resv_ctx);
> >               if (ret == -EDEADLK)
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch