linux-kernel - Re: [BUG] Circular locking dependency

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <xa1t8ut8wc2f.fsf@mina86.com>
Date:	Tue, 18 Feb 2014 18:44:08 +0100
From:	Michal Nazarewicz <mina86@...a86.com>
To:	Marek Szyprowski <m.szyprowski@...sung.com>,
	Russell King - ARM Linux <linux@....linux.org.uk>
Cc:	linux-arm-kernel@...ts.infradead.org,
	David Airlie <airlied@...ux.ie>,
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [BUG] Circular locking dependency - DRM/CMA/MM/hotplug/...

> On 2014-02-12 17:33, Russell King - ARM Linux wrote:
>> What if we did these changes:
>>
>> struct page *dma_alloc_from_contiguous(struct device *dev, int count,
>>                                         unsigned int align)
>> {
>> ...
>>          mutex_lock(&cma_mutex);
>> ...
>>          for (;;) {
>>                  pageno = bitmap_find_next_zero_area(cma->bitmap, cma->count,
>>                                                      start, count, mask);
>>                  if (pageno >= cma->count)
>>                          break;
>>
>>                  pfn = cma->base_pfn + pageno;
>> +               bitmap_set(cma->bitmap, pageno, count);
>> +               mutex_unlock(&cma_mutex);
>>                  ret = alloc_contig_range(pfn, pfn + count, MIGRATE_CMA);
>> +               mutex_lock(&cma_mutex);
>>                  if (ret == 0) {
>> -                       bitmap_set(cma->bitmap, pageno, count);
>>                          page = pfn_to_page(pfn);
>>                          break;
>> -               } else if (ret != -EBUSY) {
>> +		}
>> +		bitmap_clear(cma->bitmap, pageno, count);
>> +		if (ret != -EBUSY) {
>>                          break;
>>                  }
>> ...
>>          mutex_unlock(&cma_mutex);
>>          pr_debug("%s(): returned %p\n", __func__, page);
>>          return page;
>> }

Like Marek said, this will fail if two concurrent calls to
alloc_contig_range are made such that they operate on the same pageblock
(which is possible as the allocated regions do not need to be pageblock
aligned).

Another mutex could be added just for this one call, but as I understand
this would not solve the problem.

>> bool dma_release_from_contiguous(struct device *dev, struct page *pages,
>>                                   int count)
>> {
>> ...
>> +       free_contig_range(pfn, count);
>>          mutex_lock(&cma_mutex);
>>          bitmap_clear(cma->bitmap, pfn - cma->base_pfn, count);
>> -       free_contig_range(pfn, count);
>>          mutex_unlock(&cma_mutex);
>> ...
>> }

This *should* be fine.  Didn't test it.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@...gle.com>--<xmpp:mina86@...ber.org>--ooO--(_)--Ooo--


Download attachment "signature.asc" of type "application/pgp-signature" (836 bytes)