linux-kernel - Re: mm/vmscan.c:3504 walk_pte_range() error: uninitialized symbol 'dirty'.

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: <aDRfak8sX1Pf53Pg@stanley.mountain>
Date: Mon, 26 May 2025 15:32:42 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: oe-kbuild@...ts.linux.dev, Yu Zhao <yuzhao@...gle.com>, lkp@...el.com,
	oe-kbuild-all@...ts.linux.dev, linux-kernel@...r.kernel.org,
	Linux Memory Management List <linux-mm@...ck.org>
Subject: Re: mm/vmscan.c:3504 walk_pte_range() error: uninitialized symbol
 'dirty'.

On Fri, May 23, 2025 at 03:27:05PM -0700, Andrew Morton wrote:
> On Fri, 23 May 2025 13:47:54 +0300 Dan Carpenter <dan.carpenter@...aro.org> wrote:
> 
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head:   088d13246a4672bc03aec664675138e3f5bff68c
> > commit: a52dcec56c5b96250f15efbd7de3d3ea6ce863d9 mm/mglru: fix PTE-mapped large folios
> > config: sparc-randconfig-r073-20250515 (https://download.01.org/0day-ci/archive/20250515/202505152339.fBOfDPsi-lkp@intel.com/config)
> > compiler: sparc64-linux-gcc (GCC) 8.5.0
> > 
> > smatch warnings:
> > mm/vmscan.c:3504 walk_pte_range() error: uninitialized symbol 'dirty'.
> > mm/vmscan.c:3595 walk_pmd_range_locked() error: uninitialized symbol 'dirty'.
> > mm/vmscan.c:4215 lru_gen_look_around() error: uninitialized symbol 'dirty'.
> > 
> > ...
> >
> > bd74fdaea14602 Yu Zhao        2022-09-18  3484  	for (i = pte_index(start), addr = start; addr != end; i++, addr += PAGE_SIZE) {
> > bd74fdaea14602 Yu Zhao        2022-09-18  3485  		unsigned long pfn;
> > bd74fdaea14602 Yu Zhao        2022-09-18  3486  		struct folio *folio;
> > c33c794828f212 Ryan Roberts   2023-06-12  3487  		pte_t ptent = ptep_get(pte + i);
> > bd74fdaea14602 Yu Zhao        2022-09-18  3488  
> > bd74fdaea14602 Yu Zhao        2022-09-18  3489  		total++;
> > bd74fdaea14602 Yu Zhao        2022-09-18  3490  		walk->mm_stats[MM_LEAF_TOTAL]++;
> > bd74fdaea14602 Yu Zhao        2022-09-18  3491  
> > 1d4832becdc2cd Yu Zhao        2024-10-19  3492  		pfn = get_pte_pfn(ptent, args->vma, addr, pgdat);
> > bd74fdaea14602 Yu Zhao        2022-09-18  3493  		if (pfn == -1)
> > bd74fdaea14602 Yu Zhao        2022-09-18  3494  			continue;
> > bd74fdaea14602 Yu Zhao        2022-09-18  3495  
> > 798c0330c2ca07 Yu Zhao        2024-12-30  3496  		folio = get_pfn_folio(pfn, memcg, pgdat);
> > bd74fdaea14602 Yu Zhao        2022-09-18  3497  		if (!folio)
> > bd74fdaea14602 Yu Zhao        2022-09-18  3498  			continue;
> > bd74fdaea14602 Yu Zhao        2022-09-18  3499  
> > 1d4832becdc2cd Yu Zhao        2024-10-19  3500  		if (!ptep_clear_young_notify(args->vma, addr, pte + i))
> > 1d4832becdc2cd Yu Zhao        2024-10-19  3501  			continue;
> > bd74fdaea14602 Yu Zhao        2022-09-18  3502  
> > a52dcec56c5b96 Yu Zhao        2024-12-30  3503  		if (last != folio) {
> > a52dcec56c5b96 Yu Zhao        2024-12-30 @3504  			walk_update_folio(walk, last, gen, dirty);
> 
> Seems to be notabug because last==NULL on the first loop and in this
> case walk_update_folio() will immediately return without touching
> `dirty'.   But gee, I can't blame smatch from getting fooled by this.

That's true, but if walk_update_folio() we not inlined then we would
still consider this a bug.  It's undefined behavior in the C standard
to pass uninitialized variables to a function call and also the UBSan
checker will detected it as a read at runtime.

In production systems the compiler is going to set
"bool dirty = false;" at the start of the function because everyone
runs with CONFIG_CC_HAS_AUTO_VAR_INIT_ZERO=y.  Should I send a patch
which does that explicitly?

regards,
dan carpenter