Received: by 2002:a05:6a10:206:0:0:0:0 with SMTP id 6csp3269753pxj; Mon, 14 Jun 2021 19:29:58 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx1QMojjV+oK389wrrz5B9duCMgn/QbGGwKKgmsCr6BQMdr6wUzaI+s6mU12l2ox/MrdiYY X-Received: by 2002:a17:906:8319:: with SMTP id j25mr18042950ejx.479.1623724198276; Mon, 14 Jun 2021 19:29:58 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1623724198; cv=none; d=google.com; s=arc-20160816; b=05+Tt0JBd2kZQVoSvVZNDGqHK43Myg0oKm4lXQV3LDIc1+EXJ29J04G+Dz1Zybs+bY ILYB4E4/2XJUCe9zalxxYY4bLuXe7IiM86v3vFqtI7zJBYY7oxTCNR+shpjXgMEaJ3Mq ALZzke1k3ePX4LTjOSEHnkUgpYsW7UR+cBLtzFeO4NP4iNyMDf6jaeJXCzKINMJpkJMJ bC+GK95CcZ7+TWzHrm1dJT5bQd1nJp8VcnPIABOt9AT7NASFNX+glH2ztHHH/w1tgPLb tx6raoshpCG+5qUJWA7cC2i0Xo9HQ+MTtIDEC5JN7IzJHDUaayX7lei4YufLI9coB1o2 ViAA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:content-language :in-reply-to:mime-version:user-agent:date:message-id:from:references :cc:to:subject:dkim-signature; bh=bb+UZyvF103OTFB+0oPWDOXa4REu4kZb3WG5Dmt+tLs=; b=qphxNj6ZHOUBPJHzT+MOkZXGCtHl0uZUAaWnhBErFgdwEmUjF+EVC3KmrmV5an0/sN +EM/XL/QZqiNvM3KUgPSZtc7fg2BnhDOTFN3cyjiNWZPAbaS9jjux4VszNQM8A7Nn8A2 dHVmSQSyhSGUw7m1oxQqVYiATNIyuMGh3sZTf7O6wLeIAwJr9KkjuAi5uejhP6mwOaDS OR6XzgavMcBUkuGMqO0ziOsbpP6hoILyRct/3N2UuWIyG+nSWF+OBmcMB0pBOzBbzHS4 +z8w+spWirn4lMbddyO0sDH8Y+qPVJnUN01CQ9nzqzEMallAcDW0CxcaW+i0qHg9So2u ok4g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b="djhNl/bp"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id c22si8335150edj.513.2021.06.14.19.29.35; Mon, 14 Jun 2021 19:29:58 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b="djhNl/bp"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231917AbhFOC1b (ORCPT + 99 others); Mon, 14 Jun 2021 22:27:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43892 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231922AbhFOC12 (ORCPT ); Mon, 14 Jun 2021 22:27:28 -0400 Received: from mail-pf1-x436.google.com (mail-pf1-x436.google.com [IPv6:2607:f8b0:4864:20::436]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D5FD6C0613A2 for ; Mon, 14 Jun 2021 19:25:24 -0700 (PDT) Received: by mail-pf1-x436.google.com with SMTP id c12so12104951pfl.3 for ; Mon, 14 Jun 2021 19:25:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=bb+UZyvF103OTFB+0oPWDOXa4REu4kZb3WG5Dmt+tLs=; b=djhNl/bpEcNYfgOtTfUrpPCUBgvF5eX69I7/ZWGqOVvSpEWM6gcZ0URW4tIVQQnhWC 6SVFL5enr0JvmIqakrJWdsfNkQFFp9mlZ1pgp2NWAz/R+gIdABlQkbKe9MBtNV6hO7Tz tny8vYeBX8DFxcRNA71AlLIKkm8+F3IEAw9JnxyX1pdZcplCm6J3r0/jJjvh/wQXWoQL EVQoWohGLrQRT4xc2bv3StU7lOYi71HGdQo3OkAU90H3NnGvrp9hPZv2kuEFubIHOEZL gi5FxCspuhjq3QTuLxAbzwQJdI1BO4n4G2OZ9wu4yDzhTcfpQXGtNi4irnwT3r8+sZHm qerA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=bb+UZyvF103OTFB+0oPWDOXa4REu4kZb3WG5Dmt+tLs=; b=UOqww1Ir9moLSRJd38FwBN+TT/KyyC5SlMB/0eLYVpQcd2nGVda37FurUHk7cIqxYS XgqlT7z49B0VAUJ5fwzWuYZ7eI465/gDbF3dWFlIYOSMrLNp6Jgb9+DeC0dx+is+Q0pH syLn00+BxTX4cNnAYsCDFDYvgjpOPd8JOrjNIPsgHYrZGV7ciNc5Ikjt9FgeOqu8z0HF KgSNqCNoWuF/6Za7k67IqVGzN8eyGWzSAKzdVZGAzUsfW7y+9N6UePNIAVtebnRLlLBa R71jKUdINkKP5LGnfQzqJP0YtWeJcPLqbISjyBF9Gty0+kQxAARKNBU/8wgwYFybK6nc dI+A== X-Gm-Message-State: AOAM533ERO0+pA5709LU95VU1/0F7bKXP6V/eRxC4Ihy8KWIsMlTL7dR oCBmfQDIzVDF4evjtfuCfS6rC3B4H5iIXDcIw4s= X-Received: by 2002:a63:d511:: with SMTP id c17mr1255540pgg.219.1623723334490; Mon, 14 Jun 2021 19:15:34 -0700 (PDT) Received: from [192.168.1.237] ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id a9sm628494pjm.51.2021.06.14.19.15.30 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 14 Jun 2021 19:15:33 -0700 (PDT) Subject: Re: [PATCH 2/2] drm: Protect drm_master pointers in drm_lease.c To: Emil Velikov Cc: Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Dave Airlie , Daniel Vetter , Greg Kroah-Hartman , "Linux-Kernel@Vger. Kernel. Org" , ML dri-devel , Daniel Vetter , skhan@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org References: <20210612125426.6451-1-desmondcheongzx@gmail.com> <20210612125426.6451-3-desmondcheongzx@gmail.com> From: Desmond Cheong Zhi Xi Message-ID: <7bd0d514-efc6-8118-0b28-dfa0bcf5d842@gmail.com> Date: Tue, 15 Jun 2021 10:15:29 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 15/6/21 3:41 am, Emil Velikov wrote: > On Sat, 12 Jun 2021 at 13:55, Desmond Cheong Zhi Xi > wrote: >> >> This patch ensures that the device's master mutex is acquired before >> accessing pointers to struct drm_master that are subsequently >> dereferenced. Without the mutex, the struct drm_master may be freed >> concurrently by another process calling drm_setmaster_ioctl(). This >> could then lead to use-after-free errors. >> >> Reported-by: Daniel Vetter >> Signed-off-by: Desmond Cheong Zhi Xi >> --- > > > >> @@ -578,6 +594,7 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev, >> /* Hook up the fd */ >> fd_install(fd, lessee_file); >> >> + mutex_unlock(&dev->master_mutex); > > I was going to suggest pushing the unlock call further up - after the > drm_lease_create call. Although that would make the error path rather > messy, so let's keep it as-is. > > > >> @@ -662,7 +684,7 @@ int drm_mode_get_lease_ioctl(struct drm_device *dev, >> struct drm_mode_get_lease *arg = data; >> __u32 __user *object_ids = (__u32 __user *) (uintptr_t) (arg->objects_ptr); >> __u32 count_objects = arg->count_objects; >> - struct drm_master *lessee = lessee_priv->master; >> + struct drm_master *lessee; >> struct idr *object_idr; >> int count; >> void *entry; >> @@ -678,8 +700,10 @@ int drm_mode_get_lease_ioctl(struct drm_device *dev, >> >> DRM_DEBUG_LEASE("get lease for %d\n", lessee->lessee_id); >> >> + mutex_lock(&dev->master_mutex); > > As-is we're dereferencing an uninitialised pointer in DRM_DEBUG_LEASE. > Move the lock and assignment before the DRM_DEBUG_LEASE, just like > you've done in the list ioctl. > > With this fix, the patch is; > Reviewed-by: Emil Velikov > > -Emil > Good catch, thanks for the feedback Emil. I'll fix this up in a v2 patchset. Best wishes, Desmond