Comment 10 for bug 1837430

Revision history for this message
Daniel Badea (daniel.badea) wrote :

From the kernel log we have:

1. "BUG: unable to handle kernel paging request at 0000000000001118"
   Small address suggests NULL pointer dereference issue.

2. other registers with similar values:
   RAX: 0000000000001120
   RBX: 00000000000010c8

3. instruction pointer where crash occured:
   "RIP [<ffffffff8aa4c3a0>] eventpoll_release_file+0x50/0xc0"

4. instructions surrounding code that generated the page fault
   (note the < > marker surrounding last executed instruction):

  Code: 65 48 8b 04 25 28 00 00 00 48 89 45 d8 31 c0 e8 27 24 5c
        00 49 8b 06 48 89 45 d0 48 8b 45 d0 48 8d 58 a8 49 39 c6
        74 3b 0f 1f 00 <4c> 8b 6b 50 4d 8d 65 08 4c 89 e7 e8 00 24
        5c 00 48 89 de 4c 89

When disassembled, the offending code looks like:

0: 65 48 8b 04 25 28 00 mov rax,QWORD PTR gs:0x28
7: 00 00
9: 48 89 45 d8 mov QWORD PTR [rbp-0x28],rax
d: 31 c0 xor eax,eax
f: e8 27 24 5c 00 call 0x5c243b
14: 49 8b 06 mov rax,QWORD PTR [r14]
17: 48 89 45 d0 mov QWORD PTR [rbp-0x30],rax
1b: 48 8b 45 d0 mov rax,QWORD PTR [rbp-0x30]
1f: 48 8d 58 a8 lea rbx,[rax-0x58]
23: 49 39 c6 cmp r14,rax <-- while loop
26: 74 3b je 0x63
28: 0f 1f 00 nop DWORD PTR [rax]
2b: 4c 8b 6b 50 mov r13,QWORD PTR [rbx+0x50] <--- crash here
2f: 4d 8d 65 08 lea r12,[r13+0x8]
33: 4c 89 e7 mov rdi,r12
36: e8 00 24 5c 00 call 0x5c243b
3b: 48 89 de mov rsi,rbx
3e: 4c rex.WR
3f: 89 .byte 0x89

and if we can confirm the NULL pointer dereference caused by

   mov r13,QWORD PTR [rbx+0x50]

Current code (comments stripped):

  void eventpoll_release_file(struct file *file)
  {
          struct eventpoll *ep;
          struct epitem *epi;

          mutex_lock(&epmutex);
          list_for_each_entry_rcu(epi, &file->f_ep_links, fllink) {
                  ep = epi->ep;
                  mutex_lock_nested(&ep->mtx, 0);
                  ep_remove(ep, epi);
                  mutex_unlock(&ep->mtx);
          }
          mutex_unlock(&epmutex);
  }

To confirm we're looking at the same source, let's use debug symbols
to identify the byte code sequence:

1. unpack kernel sources in build workspace and note where the intermediate
   source directory is located:

    build-srpm-serial --edit kernel
    # 15:53:49 ===== Source code can be found at: /localdisk/loadbuild/jenkins/test/std/srpm_work/kernel/gits/kernel.spec/kernel-3.10.0-957.12.2.el7/linux-3.10.0-957.12.2.el7

2. use kernel-debuginfo rpm to disassemble eventpoll_release_file:

    cd $MY_WORKSPACE
    KSRC_DIR=/localdisk/loadbuild/jenkins/test/std/srpm_work/kernel/gits/kernel.spec/kernel-3.10.0-957.12.2.el7/linux-3.10.0-957.12.2.el7
    KDBG_RPM=$(realpath ./std/rpmbuild/RPMS/kernel-debuginfo-3.10.0-957.12.2.el7.2.tis.x86_64.rpm)
    VMLINUX=$(rpm2cpio ${KDBG_RPM} | cpio -t | grep vmlinux)
    cd /tmp
    rpm2cpio ${KDBG_RPM} | cpio -idv ${VMLINUX}
    gdb --batch --command=/dev/stdin ${VMLINUX} <<EOF
    set width 0
    set height 0
    set verbose off
    set disassembly-flavor intel
    directory ${KSRC_DIR}
    disassemble/m eventpoll_release_file
    EOF

we get:
  ...
  933 mutex_lock(&epmutex);
    <+16>: mov rdi,0xffffffff81e82b80
    <+52>: call 0xffffffff8180e7b0 <mutex_lock>

  934 list_for_each_entry_rcu(epi, &file->f_ep_links, fllink) {
    <+23>: add r14,0xb0
    <+57>: mov rax,QWORD PTR [r14]
    <+60>: mov QWORD PTR [rbp-0x30],rax
    <+64>: mov rax,QWORD PTR [rbp-0x30]
    <+68>: lea rbx,[rax-0x58]
    <+72>: cmp r14,rax
    <+75>: je 0xffffffff8124c3d8 <eventpoll_release_file+136>
    <+77>: nop DWORD PTR [rax]
    <+115>: mov rax,QWORD PTR [rbx+0x58]
    <+119>: mov QWORD PTR [rbp-0x30],rax
    <+123>: mov rax,QWORD PTR [rbp-0x30]
    <+127>: lea rbx,[rax-0x58]
    <+131>: cmp r14,rax
    <+134>: jne 0xffffffff8124c3a0 <eventpoll_release_file+80>

  935 ep = epi->ep;
    <+80>: mov r13,QWORD PTR [rbx+0x50]
  ...

where we can identify "ep = epi-> ep" as the line that caused the page
fault.

Git blame in upstream source code fs/eventpoll.c reveals a bug fix in this
area of code for a use-after-free issue:

https://github.com/torvalds/linux/commit/ebe06187bf2aec10d537ce4595e416035367d703

> From ebe06187bf2aec10d537ce4595e416035367d703 Mon Sep 17 00:00:00 2001
> From: Konstantin Khlebnikov <email address hidden>
> Date: Tue, 17 Jun 2014 06:58:05 +0400
> Subject: [PATCH] epoll: fix use-after-free in eventpoll_release_file

> This fixes use-after-free of epi->fllink.next inside list loop macro.
> This loop actually releases elements in the body. The list is
> rcu-protected but here we cannot hold rcu_read_lock because we need to
> lock mutex inside.

> The obvious solution is to use list_for_each_entry_safe(). RCU-ness
> isn't essential because nobody can change this list under us, it's final
> fput for this file.

> The bug was introduced by ae10b2b4eb01 ("epoll: optimize EPOLL_CTL_DEL
> using rcu")

> Signed-off-by: Konstantin Khlebnikov <email address hidden>
> Reported-by: Cyrill Gorcunov <email address hidden>
> Cc: Stable <email address hidden> # 3.13+
> Cc: Sasha Levin <email address hidden>
> Cc: Jason Baron <email address hidden>
> Signed-off-by: Linus Torvalds <email address hidden>
> ---
> fs/eventpoll.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

> diff --git a/fs/eventpoll.c b/fs/eventpoll.c
> index b73e0621ce9e79..b10b48c2a7afaf 100644
> --- a/fs/eventpoll.c
> +++ b/fs/eventpoll.c
> @@ -910,7 +910,7 @@ static const struct file_operations eventpoll_fops = {
> void eventpoll_release_file(struct file *file)
> {
> struct eventpoll *ep;
> - struct epitem *epi;
> + struct epitem *epi, *next;

> /*
> * We don't want to get "file->f_lock" because it is not
> @@ -926,7 +926,7 @@ void eventpoll_release_file(struct file *file)
> * Besides, ep_remove() acquires the lock, so we can't hold it here.
> */
> mutex_lock(&epmutex);
> - list_for_each_entry_rcu(epi, &file->f_ep_links, fllink) {
> + list_for_each_entry_safe(epi, next, &file->f_ep_links, fllink) {
> ep = epi->ep;
> mutex_lock_nested(&ep->mtx, 0);
> ep_remove(ep, epi);

My guess is our issue is similar and should be fixed by back-porting
the upstream patch.