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]
Date:	Fri, 05 Jul 2013 12:06:19 +0800
From:	Chen Gang <gang.chen@...anux.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	Mel Gorman <mgorman@...e.de>, hannes@...xchg.org, riel@...hat.com,
	mhocko@...e.cz, linux-mm@...ck.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mm/vmscan.c: 'lru' may be used without initialized after
 the patch "3abf380..." in next-20130607 tree

On 06/19/2013 03:19 PM, Chen Gang wrote:
> On 06/19/2013 03:10 PM, Andrew Morton wrote:
>> On Wed, 19 Jun 2013 14:55:13 +0800 Chen Gang <gang.chen@...anux.com> wrote:
>>
>>>>
>>>> 'lru' may be used without initialized, so need regressing part of the
>>>> related patch.
>>>>
>>>> The related patch:
>>>>   "3abf380 mm: remove lru parameter from __lru_cache_add and lru_cache_add_lru"
>>>>
>>>> ...
>>>>
>>>> --- a/mm/vmscan.c
>>>> +++ b/mm/vmscan.c
>>>> @@ -595,6 +595,7 @@ redo:
>>>>  		 * unevictable page on [in]active list.
>>>>  		 * We know how to handle that.
>>>>  		 */
>>>> +		lru = !!TestClearPageActive(page) + page_lru_base_type(page);
>>>>  		lru_cache_add(page);
>>>>  	} else {
>>>>  		/*
>> That looks right.  Why the heck didn't gcc-4.4.4 (at least) warn about it?
>>
> 
> Sorry I don't know either, I find it by reading code, this time.
> 
> It is really necessary to continue analyzing why. In 2nd half of 2013, I
> have planned to make some patches outside kernel but related with kernel
> (e.g. LTP, gcc patches).
> 
> This kind of issue is a good chance for me to start in 2nd half of 2013
> (start from next month).
> 
> So if no others reply for it, I will start analyzing it in the next
> month, and plan to finish within a month (before 2013-07-31).
> 
> 
> Welcome additional suggestions or completions.
> 
> Thanks.
> 

Under gcc 4.7.2 20120921 (Red Hat 4.7.2-2) also cause this issue.

The root cause is:

  for putback_lur_page() in mm/vmscan.c for next-20130621 tree.
  the compiler assumes "lru == LRU_UNEVICTABLE" instead of report warnings (uninitializing lru)

The details are below, and the related info and warn are in
attachments, please check, thanks.

Next, I will compile gcc compiler with the gcc latest code, if also has
this issue, I should communicate with gcc mailing list for it.

Thanks.

------------------------------analyzing begin---------------------------------

/* source code in mm/vmscan.c for next-20130621 */

 580 void putback_lru_page(struct page *page)
 581 {
 582         int lru;
 583         int was_unevictable = PageUnevictable(page);
 584 
 585         VM_BUG_ON(PageLRU(page));
 586 
 587 redo:
 588         ClearPageUnevictable(page);
 589 
 590         if (page_evictable(page)) {
 591                 /*
 592                  * For evictable pages, we can use the cache.
 593                  * In event of a race, worst case is we end up with an
 594                  * unevictable page on [in]active list.
 595                  * We know how to handle that.
 596                  */
 597                 lru_cache_add(page);
 598         } else {
 599                 /*
 600                  * Put unevictable pages directly on zone's unevictable
 601                  * list.
 602                  */
 603                 lru = LRU_UNEVICTABLE;
 604                 add_page_to_unevictable_list(page);
 605                 /*
 606                  * When racing with an mlock or AS_UNEVICTABLE clearing
 607                  * (page is unlocked) make sure that if the other thread
 608                  * does not observe our setting of PG_lru and fails
 609                  * isolation/check_move_unevictable_pages,
 610                  * we see PG_mlocked/AS_UNEVICTABLE cleared below and move
 611                  * the page back to the evictable list.
 612                  *
 613                  * The other side is TestClearPageMlocked() or shmem_lock().
 614                  */
 615                 smp_mb();
 616         }
 617 
 618         /*
 619          * page's status can change while we move it among lru. If an evictable
 620          * page is on unevictable list, it never be freed. To avoid that,
 621          * check after we added it to the list, again.
 622          */
 623         if (lru == LRU_UNEVICTABLE && page_evictable(page)) {
 624                 if (!isolate_lru_page(page)) {
 625                         put_page(page);
 626                         goto redo;
 627                 }
 628                 /* This means someone else dropped this page from LRU
 629                  * So, it will be freed or putback to LRU again. There is
 630                  * nothing to do here.
 631                  */
 632         }
 633 
 634         if (was_unevictable && lru != LRU_UNEVICTABLE)
 635                 count_vm_event(UNEVICTABLE_PGRESCUED);
 636         else if (!was_unevictable && lru == LRU_UNEVICTABLE)
 637                 count_vm_event(UNEVICTABLE_PGCULLED);
 638 
 639         put_page(page);         /* drop ref from isolate */
 640 }


/*
 * Related disassemble code:
 *   make defconfig under x86_64 PC.
 *   make menuconfig (choose "Automount devtmpfs at /dev..." and KGDB)
 *   make V=1 EXTRA_CFLAGS=-W (not find related warnings, ref warn.log in attachment)
 *   objdump -d vmlinux > vmlinux.S
 *   vi vmlinux.S
 *
 * The issue is: compiler assumes "lru == LRU_UNEVICTABLE" instead of report warnings (uninitializing lru)
 */

ffffffff810ffda0 <putback_lru_page>:
ffffffff810ffda0:	55                   	push   %rbp
ffffffff810ffda1:	48 89 e5             	mov    %rsp,%rbp
ffffffff810ffda4:	41 55                	push   %r13
ffffffff810ffda6:	41 54                	push   %r12
ffffffff810ffda8:	4c 8d 67 02          	lea    0x2(%rdi),%r12		; %r12 for ClearPageUnevictable(page);
ffffffff810ffdac:	53                   	push   %rbx
ffffffff810ffdad:	48 89 fb             	mov    %rdi,%rbx		; %rbx = page
ffffffff810ffdb0:	48 83 ec 08          	sub    $0x8,%rsp		; for lru, was_unevictable, but not used.

ffffffff810ffdb4:	4c 8b 2f             	mov    (%rdi),%r13		; %r13 = "was_unevictable = PageUnevictable(page);"
ffffffff810ffdb7:	49 c1 ed 14          	shr    $0x14,%r13
ffffffff810ffdbb:	41 83e5 01          	and    $0x1,%r13d
ffffffff810ffdbf:	90                   	nop

/* redo */
ffffffff810ffdc0:	f0 41 80 24 24 ef    	lock andb $0xef,(%r12)		; ClearPageUnevictable(page);

/* if (page_evictable(page)) { */
ffffffff810ffdc6:	48 89 df             	mov    %rbx,%rdi
ffffffff810ffdc9:	e8 92 ff ff ff       	callq  ffffffff810ffd60 <page_evictable>
ffffffff810ffdce:	85 c0                	test   %eax,%eax
ffffffff810ffdd0:	48 89 df             	mov    %rbx,%rdi
ffffffff810ffdd3:	74 0b                	je     ffffffff810ffde0 <putback_lru_page+0x40>

ffffffff810ffdd5:	e8 96 c5 ff ff       	callq  ffffffff810fc370 <lru_cache_add>
ffffffff810ffdda:	eb 0c                	jmp    ffffffff810ffde8 <putback_lru_page+0x48>
ffffffff810ffddc:	0f 1f 40 00          	nopl   0x0(%rax)

/* } else { */
						; assume lru == LRU_UNEVICTABLE
ffffffff810ffde0:	e8 ab c5 ff ff       	callq  ffffffff810fc390 <add_page_to_unevictable_list>
ffffffff810ffde5:	0f ae f0             	mfence 

/* } */

/* if (lru == LRU_UNEVICTABLE && page_evictable(page)) { */
ffffffff810ffde8:	48 89 df             	mov    %rbx,%rdi
ffffffff810ffdeb:	0f 1f 44 00 00       	nopl   0x0(%rax,%rax,1)
ffffffff810ffdf0:	e8 6b ff ff ff       	callq  ffffffff810ffd60 <page_evictable>
ffffffff810ffdf5:	85 c0                	test   %eax,%eax
ffffffff810ffdf7:	74 1f                	je     ffffffff810ffe18 <putback_lru_page+0x78>  ; assume lru == LRU_UNEVICTABLE

ffffffff810ffdf9:	48 89 df             	mov    %rbx,%rdi
ffffffff810ffdfc:	e8 0f fb ff ff       	callq  ffffffff810ff910 <isolate_lru_page>
ffffffff810ffe01:	85 c0                	test   %eax,%eax
ffffffff810ffe03:	75 13                	jne    ffffffff810ffe18 <putback_lru_page+0x78>
ffffffff810ffe05:	48 89 df             	mov    %rbx,%rdi
ffffffff810ffe08:	e8 93 c0 ff ff       	callq  ffffffff810fbea0 <put_page>
ffffffff810ffe0d:	0f 1f 00             	nopl   (%rax)
ffffffff810ffe10:	eb ae                	jmp    ffffffff810ffdc0 <putback_lru_page+0x20>	; goto redo;
ffffffff810ffe12:	66 0f 1f 44 00 00    	nopw   0x0(%rax,%rax,1)

/* } */

/* if (was_unevictable && lru != LRU_UNEVICTABLE) */
	/* skip... */

/* else if (!was_unevictable && lru == LRU_UNEVICTABLE) */
ffffffff810ffe18:	4d 85 ed             	test   %r13,%r13		; !was_unevictable, and assume lru == LRU_UNEVICTABLE
ffffffff810ffe1b:	75 09                	jne    ffffffff810ffe26 <putback_lru_page+0x86>
ffffffff810ffe1d:	65 48 ff 04 25 68 f0 	incq   %gs:0xf068	; it is for count_vm_event(UNEVICTABLE_PGCULLED)
									; and "incq   %gs:0xf078" is for count_vm_event(UNEVICTABLE_PGRESCUED)
ffffffff810ffe24:	00 00 

/* put_page(); */
ffffffff810ffe26:	48 89 df             	mov    %rbx,%rdi
ffffffff810ffe29:	e8 72 c0 ff ff       	callq  ffffffff810fbea0 <put_page>
ffffffff810ffe2e:	48 83 c4 08          	add    $0x8,%rsp
ffffffff810ffe32:	5b                   	pop    %rbx
ffffffff810ffe33:	41 5c                	pop    %r12
ffffffff810ffe35:	41 5d                	pop    %r13
ffffffff810ffe37:	5d                   	pop    %rbp
ffffffff810ffe38:	c3                   	retq   
ffffffff810ffe39:	0f 1f 80 00 00 00 00 	nopl   0x0(%rax)


------------------------------analyzing end-----------------------------------



Thanks.
-- 
Chen Gang

View attachment "info.log" of type "text/x-log" (2355 bytes)

View attachment "warn.log" of type "text/x-log" (101135 bytes)

Powered by blists - more mailing lists