Message ID | 20190922074900.853-1-yuq825@gmail.com (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers |
Received: from vger.kernel.org ([209.132.180.67]) by www.linuxtv.org with esmtp (Exim 4.84_2) (envelope-from <linux-media-owner@vger.kernel.org>) id 1iBwcy-0008HS-5Z; Sun, 22 Sep 2019 07:49:52 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727789AbfIVHtu (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Sun, 22 Sep 2019 03:49:50 -0400 Received: from mail-pf1-f195.google.com ([209.85.210.195]:33773 "EHLO mail-pf1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727785AbfIVHtu (ORCPT <rfc822;linux-media@vger.kernel.org>); Sun, 22 Sep 2019 03:49:50 -0400 Received: by mail-pf1-f195.google.com with SMTP id q10so7151458pfl.0 for <linux-media@vger.kernel.org>; Sun, 22 Sep 2019 00:49:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=/P04TVQeG2TqtKqnh5NeUjgcAZkp2eeIEEO+pcLFimI=; b=M/xx3S0fObzJAC7Q0yLbKqcZqa5MoT9YYx9XUcR4V8ItKFBQYE/QwdqzrWkmKANvGN MYuC9n+WDjYmQIm1JImn4++yj8Nv/FD2I2Atm4bEw2L//p4BP0BpQ3Z4PEnPc8wdWjCi SR8sH/6I6+5o+r/u6MOnsKtg1ZDUEUBX8HyJpspkTtzOgGSXFCqPgWU60BrMRpv5185L wKDtpTgqYgr3MJU3/3M2EIDNSvivtVl7CJOxESGXrBB2xDua2+wbM2fFC8YLYVflWWbV SqsQwDPN1muzH+wMK7WSP0UiVhamJaM6a82sSoFobxf9cjeuaZtsT7HdkC2gA3JtVsqp 1J3g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=/P04TVQeG2TqtKqnh5NeUjgcAZkp2eeIEEO+pcLFimI=; b=bIABmlmKL9DaHr07bc+Dc4MSjbM74WfMhgnk8AbNtr8V9+fyxlgYIaFp2+4AuETAiz ArClyhpTtZ4IGpOujxbjCq3NH5B3Adat5Gi/NA250pH3DDRb8cANLHLub8EvUIeUxc0G JK7x/9+SodEGkLPveQLiG+ADogZyGS/BrKjXL6a03Z42TyQ4VVINdxlPX2fzgCfYBzve nkiK9NV4VYf++OVzA8L60nUY+p5urzA1mfkeHO/zAouQhtUD+vJ1ttOp4MLFxtkmf673 ZAGG/wQGop92XZiaNSvxUYNl8uTHaVAPSdjYsJp3SXAbHcgD9LgoPKAgPSWF+QZPKSOJ yqYw== X-Gm-Message-State: APjAAAW0CQ9owB7+sbTIkGv6gCMluVkREMfmvjDnsq1rQbklBN5f+eSK zP2Mor7u7FOqypygbpunrCA= X-Google-Smtp-Source: APXvYqx9WFfqi0tOd1jiZXhwnRWHa9sevld2/rp0T8BsVLUgUcSnQPf8YghWXMs7gYxMmchxrZtI6w== X-Received: by 2002:a63:790b:: with SMTP id u11mr7725989pgc.273.1569138588365; Sun, 22 Sep 2019 00:49:48 -0700 (PDT) Received: from yuq-Aspire-4738G.lan (ah.ptr230.ptrcloud.net. [153.122.161.8]) by smtp.gmail.com with ESMTPSA id f188sm6194234pfa.170.2019.09.22.00.49.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 22 Sep 2019 00:49:47 -0700 (PDT) From: Qiang Yu <yuq825@gmail.com> To: dri-devel@lists.freedesktop.org Cc: lima@lists.freedesktop.org, Sumit Semwal <sumit.semwal@linaro.org>, linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org, Qiang Yu <yuq825@gmail.com>, =?utf-8?q?Christian_K=C3=B6nig?= <christian.koenig@amd.com> Subject: [PATCH] dma-buf/resv: fix exclusive fence get Date: Sun, 22 Sep 2019 15:49:00 +0800 Message-Id: <20190922074900.853-1-yuq825@gmail.com> X-Mailer: git-send-email 2.17.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org |
Series |
dma-buf/resv: fix exclusive fence get
|
|
Commit Message
Qiang Yu
Sept. 22, 2019, 7:49 a.m. UTC
This causes kernel crash when testing lima driver.
Cc: Christian König <christian.koenig@amd.com>
Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit")
Signed-off-by: Qiang Yu <yuq825@gmail.com>
---
drivers/dma-buf/dma-resv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
Quoting Qiang Yu (2019-09-22 08:49:00) > This causes kernel crash when testing lima driver. > > Cc: Christian König <christian.koenig@amd.com> > Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") > Signed-off-by: Qiang Yu <yuq825@gmail.com> > --- > drivers/dma-buf/dma-resv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > index 42a8f3f11681..709002515550 100644 > --- a/drivers/dma-buf/dma-resv.c > +++ b/drivers/dma-buf/dma-resv.c > @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, > if (pfence_excl) > *pfence_excl = fence_excl; > else if (fence_excl) > - shared[++shared_count] = fence_excl; > + shared[shared_count++] = fence_excl; Oops. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
Quoting Chris Wilson (2019-09-22 13:17:19) > Quoting Qiang Yu (2019-09-22 08:49:00) > > This causes kernel crash when testing lima driver. > > > > Cc: Christian König <christian.koenig@amd.com> > > Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") > > Signed-off-by: Qiang Yu <yuq825@gmail.com> > > --- > > drivers/dma-buf/dma-resv.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > > index 42a8f3f11681..709002515550 100644 > > --- a/drivers/dma-buf/dma-resv.c > > +++ b/drivers/dma-buf/dma-resv.c > > @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, > > if (pfence_excl) > > *pfence_excl = fence_excl; > > else if (fence_excl) > > - shared[++shared_count] = fence_excl; > > + shared[shared_count++] = fence_excl; > > Oops. > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Applied, thanks for the fix. -Chris
On Sun, Sep 22, 2019 at 2:08 PM Qiang Yu <yuq825@gmail.com> wrote: > > This causes kernel crash when testing lima driver. > > Cc: Christian König <christian.koenig@amd.com> > Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") > Signed-off-by: Qiang Yu <yuq825@gmail.com> Selftest for this would be lovely, now that the basic infrastructure is in place ... -Daniel > --- > drivers/dma-buf/dma-resv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > index 42a8f3f11681..709002515550 100644 > --- a/drivers/dma-buf/dma-resv.c > +++ b/drivers/dma-buf/dma-resv.c > @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, > if (pfence_excl) > *pfence_excl = fence_excl; > else if (fence_excl) > - shared[++shared_count] = fence_excl; > + shared[shared_count++] = fence_excl; > > if (!shared_count) { > kfree(shared); > -- > 2.17.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Am 30.09.19 um 09:22 schrieb Daniel Vetter: > On Sun, Sep 22, 2019 at 2:08 PM Qiang Yu <yuq825@gmail.com> wrote: >> This causes kernel crash when testing lima driver. >> >> Cc: Christian König <christian.koenig@amd.com> >> Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") >> Signed-off-by: Qiang Yu <yuq825@gmail.com> > Selftest for this would be lovely, now that the basic infrastructure > is in place ... What do you have in mind? I wouldn't even know where to start to write an unit test for this. Christian. > -Daniel > >> --- >> drivers/dma-buf/dma-resv.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c >> index 42a8f3f11681..709002515550 100644 >> --- a/drivers/dma-buf/dma-resv.c >> +++ b/drivers/dma-buf/dma-resv.c >> @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, >> if (pfence_excl) >> *pfence_excl = fence_excl; >> else if (fence_excl) >> - shared[++shared_count] = fence_excl; >> + shared[shared_count++] = fence_excl; >> >> if (!shared_count) { >> kfree(shared); >> -- >> 2.17.1 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > >
On Mon, Sep 30, 2019 at 08:57:32AM +0000, Koenig, Christian wrote: > Am 30.09.19 um 09:22 schrieb Daniel Vetter: > > On Sun, Sep 22, 2019 at 2:08 PM Qiang Yu <yuq825@gmail.com> wrote: > >> This causes kernel crash when testing lima driver. > >> > >> Cc: Christian König <christian.koenig@amd.com> > >> Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") > >> Signed-off-by: Qiang Yu <yuq825@gmail.com> > > Selftest for this would be lovely, now that the basic infrastructure > > is in place ... > > What do you have in mind? I wouldn't even know where to start to write > an unit test for this. 1. set a few fences (both excl + shared) in a dma_resv 2. get them 3. check that we got them all 4. notice that the exlusive fence isn't actually in the array (because we increment the index before storing, so the exclusive fence ended past the array). For robustness the test should check that the fences are listed in any order, not just the one the current implementation gives you. I guess the actual crash happens when we're unlucky and overflow the allocation, which is probably more rare. But KASAN should help catch that too (run that in your CI if you don't do that yet, it's pretty impressive). Or am I totally misunderstanding what's going wrong here? -Daniel > > Christian. > > > -Daniel > > > >> --- > >> drivers/dma-buf/dma-resv.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > >> index 42a8f3f11681..709002515550 100644 > >> --- a/drivers/dma-buf/dma-resv.c > >> +++ b/drivers/dma-buf/dma-resv.c > >> @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, > >> if (pfence_excl) > >> *pfence_excl = fence_excl; > >> else if (fence_excl) > >> - shared[++shared_count] = fence_excl; > >> + shared[shared_count++] = fence_excl; > >> > >> if (!shared_count) { > >> kfree(shared); > >> -- > >> 2.17.1 > >> > >> _______________________________________________ > >> dri-devel mailing list > >> dri-devel@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > >
Hi Chris, This fix has been pushed to drm-misc-next for a while. But Linux 5.4-rc kernels still does not have this fix. Should it be also pushed to drm-misc-fixes? Thanks, Qiang On Sun, Sep 22, 2019 at 8:50 PM Chris Wilson <chris@chris-wilson.co.uk> wrote: > > Quoting Chris Wilson (2019-09-22 13:17:19) > > Quoting Qiang Yu (2019-09-22 08:49:00) > > > This causes kernel crash when testing lima driver. > > > > > > Cc: Christian König <christian.koenig@amd.com> > > > Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") > > > Signed-off-by: Qiang Yu <yuq825@gmail.com> > > > --- > > > drivers/dma-buf/dma-resv.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > > > index 42a8f3f11681..709002515550 100644 > > > --- a/drivers/dma-buf/dma-resv.c > > > +++ b/drivers/dma-buf/dma-resv.c > > > @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, > > > if (pfence_excl) > > > *pfence_excl = fence_excl; > > > else if (fence_excl) > > > - shared[++shared_count] = fence_excl; > > > + shared[shared_count++] = fence_excl; > > > > Oops. > > > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > Applied, thanks for the fix. > -Chris
Hi Qiang, oh, good point. Yes it certainly should. Looks like I accidentally pushed it to the wrong branch. Thanks, Christian. Am 10.10.19 um 16:27 schrieb Qiang Yu: > Hi Chris, > > This fix has been pushed to drm-misc-next for a while. But Linux > 5.4-rc kernels still does not have this fix. > Should it be also pushed to drm-misc-fixes? > > Thanks, > Qiang > > > On Sun, Sep 22, 2019 at 8:50 PM Chris Wilson <chris@chris-wilson.co.uk> wrote: >> Quoting Chris Wilson (2019-09-22 13:17:19) >>> Quoting Qiang Yu (2019-09-22 08:49:00) >>>> This causes kernel crash when testing lima driver. >>>> >>>> Cc: Christian König <christian.koenig@amd.com> >>>> Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") >>>> Signed-off-by: Qiang Yu <yuq825@gmail.com> >>>> --- >>>> drivers/dma-buf/dma-resv.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c >>>> index 42a8f3f11681..709002515550 100644 >>>> --- a/drivers/dma-buf/dma-resv.c >>>> +++ b/drivers/dma-buf/dma-resv.c >>>> @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, >>>> if (pfence_excl) >>>> *pfence_excl = fence_excl; >>>> else if (fence_excl) >>>> - shared[++shared_count] = fence_excl; >>>> + shared[shared_count++] = fence_excl; >>> Oops. >>> >>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> >> Applied, thanks for the fix. >> -Chris
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 42a8f3f11681..709002515550 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, if (pfence_excl) *pfence_excl = fence_excl; else if (fence_excl) - shared[++shared_count] = fence_excl; + shared[shared_count++] = fence_excl; if (!shared_count) { kfree(shared);