Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752209AbdLUIuW (ORCPT ); Thu, 21 Dec 2017 03:50:22 -0500 Received: from mail-wm0-f66.google.com ([74.125.82.66]:36783 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751108AbdLUIuT (ORCPT ); Thu, 21 Dec 2017 03:50:19 -0500 X-Google-Smtp-Source: ACJfBouTjkgkvd8qg9XxsV8lk8o2TfMJD6BSqijAfdvl6Pdz8UaXhpGtHJLEtCsmV1uzv8wMEVAfTg== Date: Thu, 21 Dec 2017 09:50:15 +0100 From: Daniel Vetter To: Keith Packard Cc: linux-kernel@vger.kernel.org, Dave Airlie , Daniel Vetter , dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm: move lease init after validation in drm_lease_create Message-ID: <20171221085015.GQ26573@phenom.ffwll.local> Mail-Followup-To: Keith Packard , linux-kernel@vger.kernel.org, Dave Airlie , dri-devel@lists.freedesktop.org References: <20171221065424.1304-1-keithp@keithp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171221065424.1304-1-keithp@keithp.com> X-Operating-System: Linux phenom 4.13.0-1-amd64 User-Agent: Mutt/1.9.1 (2017-09-22) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2315 Lines: 69 On Wed, Dec 20, 2017 at 10:54:24PM -0800, Keith Packard wrote: > Patch bd36d3bab2e3d08f80766c86487090dbceed4651 fixed a deadlock in the > failure path of drm_lease_create. This made the partially initialized > lease object visible for a short window of time. > > To avoid having the lessee state appear transiently, I've rearranged > the code so that the lessor fields are not filled in until the > parameters are all validated and the function will succeed. > > Signed-off-by: Keith Packard Good catch, sorry I missed that. And registering the object as the very last thing is definitely the safe pattern. Applied, thanks for your patch. -Daniel > --- > drivers/gpu/drm/drm_lease.c | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c > index 59849f02e2ad..1402c0e71b03 100644 > --- a/drivers/gpu/drm/drm_lease.c > +++ b/drivers/gpu/drm/drm_lease.c > @@ -220,17 +220,6 @@ static struct drm_master *drm_lease_create(struct drm_master *lessor, struct idr > > mutex_lock(&dev->mode_config.idr_mutex); > > - /* Insert the new lessee into the tree */ > - id = idr_alloc(&(drm_lease_owner(lessor)->lessee_idr), lessee, 1, 0, GFP_KERNEL); > - if (id < 0) { > - error = id; > - goto out_lessee; > - } > - > - lessee->lessee_id = id; > - lessee->lessor = drm_master_get(lessor); > - list_add_tail(&lessee->lessee_list, &lessor->lessees); > - > idr_for_each_entry(leases, entry, object) { > error = 0; > if (!idr_find(&dev->mode_config.crtc_idr, object)) > @@ -246,6 +235,17 @@ static struct drm_master *drm_lease_create(struct drm_master *lessor, struct idr > } > } > > + /* Insert the new lessee into the tree */ > + id = idr_alloc(&(drm_lease_owner(lessor)->lessee_idr), lessee, 1, 0, GFP_KERNEL); > + if (id < 0) { > + error = id; > + goto out_lessee; > + } > + > + lessee->lessee_id = id; > + lessee->lessor = drm_master_get(lessor); > + list_add_tail(&lessee->lessee_list, &lessor->lessees); > + > /* Move the leases over */ > lessee->leases = *leases; > DRM_DEBUG_LEASE("new lessee %d %p, lessor %d %p\n", lessee->lessee_id, lessee, lessor->lessee_id, lessor); > -- > 2.15.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch