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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZwPg4znfu2yn2Qqw@x1n>
Date: Mon, 7 Oct 2024 09:23:47 -0400
From: Peter Xu <peterx@...hat.com>
To: Matthew Wilcox <willy@...radead.org>
Cc: manas18244@...td.ac.in, Andrew Morton <akpm@...ux-foundation.org>,
	Shuah Khan <skhan@...uxfoundation.org>,
	Anup Sharma <anupnewsmail@...il.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org,
	syzbot+093d096417e7038a689b@...kaller.appspotmail.com
Subject: Re: [PATCH v3] Fixes: null pointer dereference in
 pfnmap_lockdep_assert

On Fri, Oct 04, 2024 at 04:17:42PM +0100, Matthew Wilcox wrote:
> On Fri, Oct 04, 2024 at 07:15:48PM +0530, Manas via B4 Relay wrote:
> > +++ b/mm/memory.c
> > @@ -6346,10 +6346,10 @@ static inline void pfnmap_args_setup(struct follow_pfnmap_args *args,
> >  static inline void pfnmap_lockdep_assert(struct vm_area_struct *vma)
> >  {
> >  #ifdef CONFIG_LOCKDEP
> > -	struct address_space *mapping = vma->vm_file->f_mapping;
> > +	struct address_space *mapping = vma->vm_file ? vma->vm_file->f_mapping : NULL;
> 
> Overly long and complex line.  Much simpler to write:
> 
> 	struct address_space *mapping = NULL;
> 
> 	if (vma->vm_file)
> 		mapping = vma->vm_file->f_mapping;
> 
> >  	if (mapping)
> > -		lockdep_assert(lockdep_is_held(&vma->vm_file->f_mapping->i_mmap_rwsem) ||
> > +		lockdep_assert(lockdep_is_held(&mapping->i_mmap_rwsem) ||
> >  			       lockdep_is_held(&vma->vm_mm->mmap_lock));
> >  	else
> >  		lockdep_assert(lockdep_is_held(&vma->vm_mm->mmap_lock));
> 
> This one should have been lockdep_assert_held(&vma->vm_mm->mmap_lock).
> 
> I'm not sure that the previous one is correct.  The
> lockdep_assert_held() macro is pretty careful about checking
> LOCK_STATE_NOT_HELD to avoid the LOCK_STATE_UNKNOWN possibility.
> But I'll leave that for Peter to fix.

Indeed..

Then looks like we could have quite a few other places in Linux that can
have used this wrong.. when the assert wants to check against either of the
two locks (one mutex or rcu read lock, for example) is held.

I'll send a patch after this one lands.

Thanks,

-- 
Peter Xu


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ