All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	ira.weiny@intel.com, Linux-MM <linux-mm@kvack.org>,
	miles.chen@mediatek.com, mm-commits@vger.kernel.org
Subject: Re: [patch 07/35] mm/gup: fix null pointer dereference detected by coverity
Date: Fri, 10 Apr 2020 19:53:58 -0400	[thread overview]
Message-ID: <20200410235358.GA38470@xz-x1> (raw)
In-Reply-To: <CAHk-=wjjA-Cf5uXMo4DD1i3iiDQi2wO60Xh-QCQNEZkdPwWz0g@mail.gmail.com>

On Fri, Apr 10, 2020 at 03:24:37PM -0700, Linus Torvalds wrote:
> On Fri, Apr 10, 2020 at 2:32 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > In fixup_user_fault(), it is possible that unlocked is NULL,
> > so we should test unlocked before using it.
> 
> This seems wrong.
> 
> > For example, in arch/arc/kernel/process.c, NULL is passed
> > to fixup_user_fault().
> >
> >         ret = fixup_user_fault(current, current->mm, (unsigned long) uaddr,
> >                                FAULT_FLAG_WRITE, NULL);
> 
> Yes, but it doesn't set FAULT_FLAG_ALLOW_RETRY, exactly _because_
> 'unlocked' is NULL.
> 
> Basically, retry is fundamentally tied to that "unlocked" flag. You
> can't ask for retry without also saying "please tell me if you
> unlocked the mmap_sem during the retry". So the two go hand in hand
> there.
> 
> So I think this is just coverity not understanding the rules.

Yeah that's also my understanding - VM_FAULT_RETRY should depend on
FAULT_FLAG_ALLOW_RETRY, which further depends on unlocked != NULL.

Though I'm not sure how to teach Coverity about the fact.  Maybe a
"BUG_ON(unlocked == NULL)" (which contains an unreachable() inside)
before referencing unlocked?

Thanks,

> 
> Or maybe I'm the one missing something. Did you actually see a problem?
> 
>                    Linus
> 

-- 
Peter Xu



  reply	other threads:[~2020-04-10 23:54 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-10 21:30 incoming Andrew Morton
2020-04-10 21:32 ` [patch 01/35] hfsplus: fix crash and filesystem corruption when deleting files Andrew Morton
2020-04-10 21:32 ` [patch 02/35] mm, memcg: do not high throttle allocators based on wraparound Andrew Morton
2020-04-10 21:32 ` [patch 03/35] mm, slab_common: fix a typo in comment "eariler"->"earlier" Andrew Morton
2020-04-10 21:32 ` [patch 04/35] docs: mm: slab.h: fix a broken cross-reference Andrew Morton
2020-04-10 21:32 ` [patch 05/35] mm/page_alloc.c: fix kernel-doc warning Andrew Morton
2020-04-10 21:32 ` [patch 06/35] mm/page_alloc: make pcpu_drain_mutex and pcpu_drain static Andrew Morton
2020-04-10 21:32 ` [patch 07/35] mm/gup: fix null pointer dereference detected by coverity Andrew Morton
2020-04-10 22:24   ` Linus Torvalds
2020-04-10 23:53     ` Peter Xu [this message]
2020-04-11  0:19       ` Linus Torvalds
2020-04-14  4:04     ` Miles Chen
2020-04-10 21:32 ` [patch 08/35] ocfs2: no need try to truncate file beyond i_size Andrew Morton
2020-04-10 21:32 ` [patch 09/35] mm: cma: NUMA node interface Andrew Morton
2020-04-10 21:32 ` [patch 10/35] mm: hugetlb: optionally allocate gigantic hugepages using cma Andrew Morton
2020-04-10 21:32 ` [patch 11/35] mm/mmap.c: initialize align_offset explicitly for vm_unmapped_area Andrew Morton
2020-04-10 21:32 ` [patch 12/35] mm/memory.c: refactor insert_page to prepare for batched-lock insert Andrew Morton
2020-04-10 21:32   ` Andrew Morton
2020-04-10 21:32 ` [patch 13/35] mm: bring sparc pte_index() semantics inline with other platforms Andrew Morton
2020-04-10 21:32 ` [patch 14/35] mm: define pte_index as macro for x86 Andrew Morton
2020-04-10 21:33 ` [patch 15/35] mm/memory.c: add vm_insert_pages() Andrew Morton
2020-04-10 21:33 ` [patch 16/35] mm/vma: define a default value for VM_DATA_DEFAULT_FLAGS Andrew Morton
2020-04-10 21:33   ` Andrew Morton
2020-04-10 21:33 ` [patch 17/35] mm/vma: introduce VM_ACCESS_FLAGS Andrew Morton
2020-04-10 21:33 ` [patch 18/35] mm/special: create generic fallbacks for pte_special() and pte_mkspecial() Andrew Morton
2020-04-10 21:33 ` [patch 19/35] mm/memory_hotplug: drop the flags field from struct mhp_restrictions Andrew Morton
2020-04-10 21:33 ` [patch 20/35] mm/memory_hotplug: rename mhp_restrictions to mhp_params Andrew Morton
2020-04-10 21:33 ` [patch 21/35] x86/mm: thread pgprot_t through init_memory_mapping() Andrew Morton
2020-04-10 21:33 ` [patch 22/35] x86/mm: introduce __set_memory_prot() Andrew Morton
2020-04-10 21:33 ` [patch 23/35] powerpc/mm: thread pgprot_t through create_section_mapping() Andrew Morton
2020-04-10 21:33 ` [patch 24/35] mm/memory_hotplug: add pgprot_t to mhp_params Andrew Morton
2020-04-10 21:33 ` [patch 25/35] mm/memremap: set caching mode for PCI P2PDMA memory to WC Andrew Morton
2020-04-10 21:33 ` [patch 26/35] kmod: make request_module() return an error when autoloading is disabled Andrew Morton
2020-04-10 21:33 ` [patch 27/35] fs/filesystems.c: downgrade user-reachable WARN_ONCE() to pr_warn_once() Andrew Morton
2020-04-10 21:33 ` Andrew Morton
2020-04-10 21:33 ` [patch 28/35] docs: admin-guide: document the kernel.modprobe sysctl Andrew Morton
2020-04-10 21:33 ` [patch 29/35] selftests: kmod: fix handling test numbers above 9 Andrew Morton
2020-04-10 21:33 ` [patch 30/35] selftests: kmod: test disabling module autoloading Andrew Morton
2020-04-10 21:34 ` [patch 31/35] change email address for Pali Rohár Andrew Morton
2020-04-10 21:44   ` Joe Perches
2020-04-10 21:34 ` [patch 32/35] drivers/dma/tegra20-apb-dma.c: fix platform_get_irq.cocci warnings Andrew Morton
2020-04-13 17:54   ` Jon Hunter
2020-04-10 21:34 ` [patch 33/35] fs/seq_file.c: seq_read(): add info message about buggy .next functions Andrew Morton
2020-04-10 21:34 ` [patch 34/35] kernel/gcov/fs.c: gcov_seq_next() should increase position index Andrew Morton
2020-04-10 21:34 ` [patch 35/35] ipc/util.c: sysvipc_find_ipc() " Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200410235358.GA38470@xz-x1 \
    --to=peterx@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=ira.weiny@intel.com \
    --cc=linux-mm@kvack.org \
    --cc=miles.chen@mediatek.com \
    --cc=mm-commits@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.