* [syzbot] [mm?] general protection fault in hugetlb_vma_lock_write
@ 2023-10-29 9:27 syzbot
2023-10-31 18:38 ` Rik van Riel
2023-11-01 6:36 ` [PATCH] mm/hugetlb: fix null ptr defer " Edward Adam Davis
0 siblings, 2 replies; 17+ messages in thread
From: syzbot @ 2023-10-29 9:27 UTC (permalink / raw)
To: akpm, linux-kernel, linux-mm, llvm, mike.kravetz, muchun.song,
nathan, ndesaulniers, riel, syzkaller-bugs, trix
Hello,
syzbot found the following issue on:
HEAD commit: 888cf78c29e2 Merge tag 'iommu-fix-v6.6-rc7' of git://git.k..
git tree: upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=10cbee95680000
kernel config: https://syzkaller.appspot.com/x/.config?x=9ee601744db6e780
dashboard link: https://syzkaller.appspot.com/bug?extid=6ada951e7c0f7bc8a71e
compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10fa2557680000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14285b57680000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/ca8ad4106267/disk-888cf78c.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/aac2e80e0744/vmlinux-888cf78c.xz
kernel image: https://storage.googleapis.com/syzbot-assets/b14533456815/bzImage-888cf78c.xz
The issue was bisected to:
commit bf4916922c60f43efaa329744b3eef539aa6a2b2
Author: Rik van Riel <riel@surriel.com>
Date: Fri Oct 6 03:59:07 2023 +0000
hugetlbfs: extend hugetlb_vma_lock to private VMAs
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=17d7dab5680000
final oops: https://syzkaller.appspot.com/x/report.txt?x=1437dab5680000
console output: https://syzkaller.appspot.com/x/log.txt?x=1037dab5680000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+6ada951e7c0f7bc8a71e@syzkaller.appspotmail.com
Fixes: bf4916922c60 ("hugetlbfs: extend hugetlb_vma_lock to private VMAs")
general protection fault, probably for non-canonical address 0xdffffc000000001d: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x00000000000000e8-0x00000000000000ef]
CPU: 0 PID: 5048 Comm: syz-executor139 Not tainted 6.6.0-rc7-syzkaller-00142-g888cf78c29e2 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/09/2023
RIP: 0010:__lock_acquire+0x109/0x5de0 kernel/locking/lockdep.c:5004
Code: 45 85 c9 0f 84 cc 0e 00 00 44 8b 05 c1 1e 42 0b 45 85 c0 0f 84 be 0d 00 00 48 ba 00 00 00 00 00 fc ff df 4c 89 d1 48 c1 e9 03 <80> 3c 11 00 0f 85 e8 40 00 00 49 81 3a a0 d9 5f 90 0f 84 96 0d 00
RSP: 0018:ffffc90003aa7798 EFLAGS: 00010016
RAX: ffff88807a0b9dc0 RBX: 1ffff92000754f23 RCX: 000000000000001d
RDX: dffffc0000000000 RSI: 0000000000000000 RDI: 00000000000000e8
RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000001
R10: 00000000000000e8 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff8880b9800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020000280 CR3: 00000000758bf000 CR4: 0000000000350ef0
Call Trace:
<TASK>
lock_acquire kernel/locking/lockdep.c:5753 [inline]
lock_acquire+0x1ae/0x510 kernel/locking/lockdep.c:5718
down_write+0x93/0x200 kernel/locking/rwsem.c:1573
hugetlb_vma_lock_write mm/hugetlb.c:300 [inline]
hugetlb_vma_lock_write+0xae/0x100 mm/hugetlb.c:291
__hugetlb_zap_begin+0x1e9/0x2b0 mm/hugetlb.c:5447
hugetlb_zap_begin include/linux/hugetlb.h:258 [inline]
unmap_vmas+0x2f4/0x470 mm/memory.c:1733
exit_mmap+0x1ad/0xa60 mm/mmap.c:3230
__mmput+0x12a/0x4d0 kernel/fork.c:1349
mmput+0x62/0x70 kernel/fork.c:1371
exit_mm kernel/exit.c:567 [inline]
do_exit+0x9ad/0x2a20 kernel/exit.c:861
__do_sys_exit kernel/exit.c:991 [inline]
__se_sys_exit kernel/exit.c:989 [inline]
__x64_sys_exit+0x42/0x50 kernel/exit.c:989
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7ff2b7a78ab9
Code: Unable to access opcode bytes at 0x7ff2b7a78a8f.
RSP: 002b:00007fff926ea6b8 EFLAGS: 00000246 ORIG_RAX: 000000000000003c
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007ff2b7a78ab9
RDX: 00007ff2b7ab23f3 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 000000000000cfda R08: 0000000000000000 R09: 0000000000000006
R10: 0000000000000000 R11: 0000000000000246 R12: 00007fff926ea6cc
R13: 431bde82d7b634db R14: 0000000000000001 R15: 0000000000000001
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:__lock_acquire+0x109/0x5de0 kernel/locking/lockdep.c:5004
Code: 45 85 c9 0f 84 cc 0e 00 00 44 8b 05 c1 1e 42 0b 45 85 c0 0f 84 be 0d 00 00 48 ba 00 00 00 00 00 fc ff df 4c 89 d1 48 c1 e9 03 <80> 3c 11 00 0f 85 e8 40 00 00 49 81 3a a0 d9 5f 90 0f 84 96 0d 00
RSP: 0018:ffffc90003aa7798 EFLAGS: 00010016
RAX: ffff88807a0b9dc0 RBX: 1ffff92000754f23 RCX: 000000000000001d
RDX: dffffc0000000000 RSI: 0000000000000000 RDI: 00000000000000e8
RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000001
R10: 00000000000000e8 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff8880b9800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020000280 CR3: 00000000758bf000 CR4: 0000000000350ef0
----------------
Code disassembly (best guess):
0: 45 85 c9 test %r9d,%r9d
3: 0f 84 cc 0e 00 00 je 0xed5
9: 44 8b 05 c1 1e 42 0b mov 0xb421ec1(%rip),%r8d # 0xb421ed1
10: 45 85 c0 test %r8d,%r8d
13: 0f 84 be 0d 00 00 je 0xdd7
19: 48 ba 00 00 00 00 00 movabs $0xdffffc0000000000,%rdx
20: fc ff df
23: 4c 89 d1 mov %r10,%rcx
26: 48 c1 e9 03 shr $0x3,%rcx
* 2a: 80 3c 11 00 cmpb $0x0,(%rcx,%rdx,1) <-- trapping instruction
2e: 0f 85 e8 40 00 00 jne 0x411c
34: 49 81 3a a0 d9 5f 90 cmpq $0xffffffff905fd9a0,(%r10)
3b: 0f .byte 0xf
3c: 84 .byte 0x84
3d: 96 xchg %eax,%esi
3e: 0d .byte 0xd
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
If the bug is already fixed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
If you want to overwrite bug's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the bug is a duplicate of another bug, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [syzbot] [mm?] general protection fault in hugetlb_vma_lock_write
2023-10-29 9:27 [syzbot] [mm?] general protection fault in hugetlb_vma_lock_write syzbot
@ 2023-10-31 18:38 ` Rik van Riel
2023-11-02 12:15 ` Aleksandr Nogikh
2023-11-01 6:36 ` [PATCH] mm/hugetlb: fix null ptr defer " Edward Adam Davis
1 sibling, 1 reply; 17+ messages in thread
From: Rik van Riel @ 2023-10-31 18:38 UTC (permalink / raw)
To: syzbot, akpm, linux-kernel, linux-mm, llvm, mike.kravetz,
muchun.song, nathan, ndesaulniers, syzkaller-bugs, trix
On Sun, 2023-10-29 at 02:27 -0700, syzbot wrote:
>
> commit bf4916922c60f43efaa329744b3eef539aa6a2b2
> Author: Rik van Riel <riel@surriel.com>
> Date: Fri Oct 6 03:59:07 2023 +0000
>
> hugetlbfs: extend hugetlb_vma_lock to private VMAs
>
I've been trying to reproduce the issue here, but the test
case has been running for 4+ hours now on a KVM guest, with
KASAN and CONFIG_PROVE_LOCKING enabled. No crashes yet.
I'll try adapting the config file from syzkaller so the
resulting kernel boots here, but this is not looking like
an easy reproducer so far...
The crash is also confusing me somewhat, because it looks
like hugetlb_vma_lock_write() is passing a nonsense (very small
value) resv_map->rw_sema pointer down to down_write, but the
code has some protection against that:
static inline bool __vma_private_lock(struct vm_area_struct *vma)
{
return (!(vma->vm_flags & VM_MAYSHARE)) && vma-
>vm_private_data;
}
void hugetlb_vma_lock_write(struct vm_area_struct *vma)
{
if (__vma_shareable_lock(vma)) {
struct hugetlb_vma_lock *vma_lock = vma-
>vm_private_data;
down_write(&vma_lock->rw_sema);
} else if (__vma_private_lock(vma)) {
struct resv_map *resv_map = vma_resv_map(vma);
down_write(&resv_map->rw_sema);
}
}
At fork time, vma->vm_private_data gets cleared in the child
process for MAP_PRIVATE hugetlb VMAs.
I do not see anything that would leave behind a tiny, but
non-zero value in that pointer.
I'll keep poking at this, but I don't know if it will
reproduce here.
> general protection fault, probably for non-canonical address
> 0xdffffc000000001d: 0000 [#1] PREEMPT SMP KASAN
> KASAN: null-ptr-deref in range [0x00000000000000e8-
> 0x00000000000000ef]
> CPU: 0 PID: 5048 Comm: syz-executor139 Not tainted 6.6.0-rc7-
> syzkaller-00142-g888cf78c29e2 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine,
> BIOS Google 10/09/2023
> RIP: 0010:__lock_acquire+0x109/0x5de0 kernel/locking/lockdep.c:5004
> Code: 45 85 c9 0f 84 cc 0e 00 00 44 8b 05 c1 1e 42 0b 45 85 c0 0f 84
> be 0d 00 00 48 ba 00 00 00 00 00 fc ff df 4c 89 d1 48 c1 e9 03 <80>
> 3c 11 00 0f 85 e8 40 00 00 49 81 3a a0 d9 5f 90 0f 84 96 0d 00
> RSP: 0018:ffffc90003aa7798 EFLAGS: 00010016
>
> RAX: ffff88807a0b9dc0 RBX: 1ffff92000754f23 RCX: 000000000000001d
> RDX: dffffc0000000000 RSI: 0000000000000000 RDI: 00000000000000e8
> RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000001
> R10: 00000000000000e8 R11: 0000000000000000 R12: 0000000000000000
> R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> FS: 0000000000000000(0000) GS:ffff8880b9800000(0000)
> knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000020000280 CR3: 00000000758bf000 CR4: 0000000000350ef0
> Call Trace:
> <TASK>
> lock_acquire kernel/locking/lockdep.c:5753 [inline]
> lock_acquire+0x1ae/0x510 kernel/locking/lockdep.c:5718
> down_write+0x93/0x200 kernel/locking/rwsem.c:1573
> hugetlb_vma_lock_write mm/hugetlb.c:300 [inline]
> hugetlb_vma_lock_write+0xae/0x100 mm/hugetlb.c:291
> __hugetlb_zap_begin+0x1e9/0x2b0 mm/hugetlb.c:5447
> hugetlb_zap_begin include/linux/hugetlb.h:258 [inline]
> unmap_vmas+0x2f4/0x470 mm/memory.c:1733
> exit_mmap+0x1ad/0xa60 mm/mmap.c:3230
> __mmput+0x12a/0x4d0 kernel/fork.c:1349
> mmput+0x62/0x70 kernel/fork.c:1371
> exit_mm kernel/exit.c:567 [inline]
> do_exit+0x9ad/0x2a20 kernel/exit.c:861
> __do_sys_exit kernel/exit.c:991 [inline]
> __se_sys_exit kernel/exit.c:989 [inline]
> __x64_sys_exit+0x42/0x50 kernel/exit.c:989
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
> RIP: 0033:0x7ff2b7a78ab9
> Code: Unable to access opcode bytes at 0x7ff2b7a78a8f.
> RSP: 002b:00007fff926ea6b8 EFLAGS: 00000246 ORIG_RAX:
> 000000000000003c
> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007ff2b7a78ab9
> RDX: 00007ff2b7ab23f3 RSI: 0000000000000000 RDI: 0000000000000000
> RBP: 000000000000cfda R08: 0000000000000000 R09: 0000000000000006
> R10: 0000000000000000 R11: 0000000000000246 R12: 00007fff926ea6cc
> R13: 431bde82d7b634db R14: 0000000000000001 R15: 0000000000000001
> </TASK>
> Modules linked in:
> ---[ end trace 0000000000000000 ]---
> RIP: 0010:__lock_acquire+0x109/0x5de0 kernel/locking/lockdep.c:5004
> Code: 45 85 c9 0f 84 cc 0e 00 00 44 8b 05 c1 1e 42 0b 45 85 c0 0f 84
> be 0d 00 00 48 ba 00 00 00 00 00 fc ff df 4c 89 d1 48 c1 e9 03 <80>
> 3c 11 00 0f 85 e8 40 00 00 49 81 3a a0 d9 5f 90 0f 84 96 0d 00
> RSP: 0018:ffffc90003aa7798 EFLAGS: 00010016
> RAX: ffff88807a0b9dc0 RBX: 1ffff92000754f23 RCX: 000000000000001d
> RDX: dffffc0000000000 RSI: 0000000000000000 RDI: 00000000000000e8
> RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000001
> R10: 00000000000000e8 R11: 0000000000000000 R12: 0000000000000000
> R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> FS: 0000000000000000(0000) GS:ffff8880b9800000(0000)
> knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000020000280 CR3: 00000000758bf000 CR4: 0000000000350ef0
> ----------------
> Code disassembly (best guess):
> 0: 45 85 c9 test %r9d,%r9d
> 3: 0f 84 cc 0e 00 00 je 0xed5
> 9: 44 8b 05 c1 1e 42 0b mov 0xb421ec1(%rip),%r8d #
> 0xb421ed1
> 10: 45 85 c0 test %r8d,%r8d
> 13: 0f 84 be 0d 00 00 je 0xdd7
> 19: 48 ba 00 00 00 00 00 movabs $0xdffffc0000000000,%rdx
> 20: fc ff df
> 23: 4c 89 d1 mov %r10,%rcx
> 26: 48 c1 e9 03 shr $0x3,%rcx
> * 2a: 80 3c 11 00 cmpb $0x0,(%rcx,%rdx,1) <--
> trapping instruction
> 2e: 0f 85 e8 40 00 00 jne 0x411c
> 34: 49 81 3a a0 d9 5f 90 cmpq $0xffffffff905fd9a0,(%r10)
> 3b: 0f .byte 0xf
> 3c: 84 .byte 0x84
> 3d: 96 xchg %eax,%esi
> 3e: 0d .byte 0xd
>
>
> ---
> This report is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this issue. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> For information about bisection process see:
> https://goo.gl/tpsmEJ#bisection
>
> If the bug is already fixed, let syzbot know by replying with:
> #syz fix: exact-commit-title
>
> If you want syzbot to run the reproducer, reply with:
> #syz test: git://repo/address.git branch-or-commit-hash
> If you attach or paste a git patch, syzbot will apply it before
> testing.
>
> If you want to overwrite bug's subsystems, reply with:
> #syz set subsystems: new-subsystem
> (See the list of subsystem names on the web dashboard)
>
> If the bug is a duplicate of another bug, reply with:
> #syz dup: exact-subject-of-another-report
>
> If you want to undo deduplication, reply with:
> #syz undup
>
--
All Rights Reversed.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write
2023-10-29 9:27 [syzbot] [mm?] general protection fault in hugetlb_vma_lock_write syzbot
2023-10-31 18:38 ` Rik van Riel
@ 2023-11-01 6:36 ` Edward Adam Davis
2023-11-01 14:27 ` Rik van Riel
1 sibling, 1 reply; 17+ messages in thread
From: Edward Adam Davis @ 2023-11-01 6:36 UTC (permalink / raw)
To: syzbot+6ada951e7c0f7bc8a71e
Cc: akpm, linux-kernel, linux-mm, llvm, mike.kravetz, muchun.song,
nathan, ndesaulniers, riel, syzkaller-bugs, trix
When obtaining resv_map from vma, it is necessary to simultaneously determine
the flag HPAGE_RESV_OWNER of vm_private_data.
Only when they are met simultaneously, resv_map is valid.
Reported-and-tested-by: syzbot+6ada951e7c0f7bc8a71e@syzkaller.appspotmail.com
Fixes: bf4916922c60 ("hugetlbfs: extend hugetlb_vma_lock to private VMAs")
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
include/linux/hugetlb.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 47d25a5e1933..1a3ec1aee1a3 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -1265,9 +1265,11 @@ static inline bool __vma_shareable_lock(struct vm_area_struct *vma)
return (vma->vm_flags & VM_MAYSHARE) && vma->vm_private_data;
}
+#define HPAGE_RESV_OWNER (1UL << 0)
static inline bool __vma_private_lock(struct vm_area_struct *vma)
{
- return (!(vma->vm_flags & VM_MAYSHARE)) && vma->vm_private_data;
+ return (!(vma->vm_flags & VM_MAYSHARE)) && vma->vm_private_data &&
+ ((unsigned long)vma->vm_private_data & HPAGE_RESV_OWNER);
}
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write
2023-11-01 6:36 ` [PATCH] mm/hugetlb: fix null ptr defer " Edward Adam Davis
@ 2023-11-01 14:27 ` Rik van Riel
2023-11-02 12:58 ` Edward Adam Davis
2023-11-02 13:46 ` Yin, Fengwei
0 siblings, 2 replies; 17+ messages in thread
From: Rik van Riel @ 2023-11-01 14:27 UTC (permalink / raw)
To: Edward Adam Davis, syzbot+6ada951e7c0f7bc8a71e
Cc: akpm, linux-kernel, linux-mm, llvm, mike.kravetz, muchun.song,
nathan, ndesaulniers, syzkaller-bugs, trix
On Wed, 2023-11-01 at 14:36 +0800, Edward Adam Davis wrote:
> When obtaining resv_map from vma, it is necessary to simultaneously
> determine
> the flag HPAGE_RESV_OWNER of vm_private_data.
> Only when they are met simultaneously, resv_map is valid.
>
> Reported-and-tested-by:
> syzbot+6ada951e7c0f7bc8a71e@syzkaller.appspotmail.com
> Fixes: bf4916922c60 ("hugetlbfs: extend hugetlb_vma_lock to private
> VMAs")
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> include/linux/hugetlb.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index 47d25a5e1933..1a3ec1aee1a3 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -1265,9 +1265,11 @@ static inline bool __vma_shareable_lock(struct
> vm_area_struct *vma)
> return (vma->vm_flags & VM_MAYSHARE) && vma->vm_private_data;
> }
>
> +#define HPAGE_RESV_OWNER (1UL << 0)
> static inline bool __vma_private_lock(struct vm_area_struct *vma)
> {
> - return (!(vma->vm_flags & VM_MAYSHARE)) && vma-
> >vm_private_data;
> + return (!(vma->vm_flags & VM_MAYSHARE)) && vma-
> >vm_private_data &&
> + ((unsigned long)vma->vm_private_data &
> HPAGE_RESV_OWNER);
> }
This could be cleaned up a bit by moving the HPAGE_RESV_OWNER
definition (and its friends) into hugetlb.h, as well as the
is_vma_resv_set() helper function.
Then __vma_private_lock() can just call is_vma_resv_set(),
and open coding a duplicate of the same code.
Not having duplicates of the code will make it much harder
to "miss a spot" with future changes.
I am still struggling to find a place where we might leave
HPAGE_RESV_OWNER behind on a pointer that is otherwise NULL,
but if your tests show this fixes the issue, I'm all for it :)
--
All Rights Reversed.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [syzbot] [mm?] general protection fault in hugetlb_vma_lock_write
2023-10-31 18:38 ` Rik van Riel
@ 2023-11-02 12:15 ` Aleksandr Nogikh
0 siblings, 0 replies; 17+ messages in thread
From: Aleksandr Nogikh @ 2023-11-02 12:15 UTC (permalink / raw)
To: Rik van Riel
Cc: syzbot, akpm, linux-kernel, linux-mm, llvm, mike.kravetz,
muchun.song, nathan, ndesaulniers, syzkaller-bugs, trix
On Tue, Oct 31, 2023 at 7:39 PM Rik van Riel <riel@surriel.com> wrote:
>
> On Sun, 2023-10-29 at 02:27 -0700, syzbot wrote:
> >
> > commit bf4916922c60f43efaa329744b3eef539aa6a2b2
> > Author: Rik van Riel <riel@surriel.com>
> > Date: Fri Oct 6 03:59:07 2023 +0000
> >
> > hugetlbfs: extend hugetlb_vma_lock to private VMAs
> >
>
> I've been trying to reproduce the issue here, but the test
> case has been running for 4+ hours now on a KVM guest, with
> KASAN and CONFIG_PROVE_LOCKING enabled. No crashes yet.
FWIW you may also try to use the syzbot-built kernel shared via the
"Downloadable assets" section[1]. I've just run the C repro against it
and it crashed immediately.
[ 66.222816][ T5095] general protection fault, probably for
non-canonical address 0xdffffc000000001d: 0000 [#1] PREEMPT SMP KASAN
[ 66.227224][ T5095] KASAN: null-ptr-deref in range
[0x00000000000000e8-0x00000000000000ef]
[ 66.230109][ T5095] CPU: 0 PID: 5095 Comm: repro Not tainted
6.6.0-rc7-syzkaller-00142-g888cf78c29e2 #0
[1] Here are the instructions with commands to copy-paste:
https://github.com/google/syzkaller/blob/master/docs/syzbot_assets.md
--
Aleksandr
>
> I'll try adapting the config file from syzkaller so the
> resulting kernel boots here, but this is not looking like
> an easy reproducer so far...
>
> The crash is also confusing me somewhat, because it looks
> like hugetlb_vma_lock_write() is passing a nonsense (very small
> value) resv_map->rw_sema pointer down to down_write, but the
> code has some protection against that:
>
> static inline bool __vma_private_lock(struct vm_area_struct *vma)
> {
> return (!(vma->vm_flags & VM_MAYSHARE)) && vma-
> >vm_private_data;
> }
>
> void hugetlb_vma_lock_write(struct vm_area_struct *vma)
> {
> if (__vma_shareable_lock(vma)) {
> struct hugetlb_vma_lock *vma_lock = vma-
> >vm_private_data;
>
> down_write(&vma_lock->rw_sema);
> } else if (__vma_private_lock(vma)) {
> struct resv_map *resv_map = vma_resv_map(vma);
>
> down_write(&resv_map->rw_sema);
> }
> }
>
> At fork time, vma->vm_private_data gets cleared in the child
> process for MAP_PRIVATE hugetlb VMAs.
>
> I do not see anything that would leave behind a tiny, but
> non-zero value in that pointer.
>
> I'll keep poking at this, but I don't know if it will
> reproduce here.
>
> > general protection fault, probably for non-canonical address
> > 0xdffffc000000001d: 0000 [#1] PREEMPT SMP KASAN
> > KASAN: null-ptr-deref in range [0x00000000000000e8-
> > 0x00000000000000ef]
> > CPU: 0 PID: 5048 Comm: syz-executor139 Not tainted 6.6.0-rc7-
> > syzkaller-00142-g888cf78c29e2 #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine,
> > BIOS Google 10/09/2023
> > RIP: 0010:__lock_acquire+0x109/0x5de0 kernel/locking/lockdep.c:5004
> > Code: 45 85 c9 0f 84 cc 0e 00 00 44 8b 05 c1 1e 42 0b 45 85 c0 0f 84
> > be 0d 00 00 48 ba 00 00 00 00 00 fc ff df 4c 89 d1 48 c1 e9 03 <80>
> > 3c 11 00 0f 85 e8 40 00 00 49 81 3a a0 d9 5f 90 0f 84 96 0d 00
> > RSP: 0018:ffffc90003aa7798 EFLAGS: 00010016
> >
> > RAX: ffff88807a0b9dc0 RBX: 1ffff92000754f23 RCX: 000000000000001d
> > RDX: dffffc0000000000 RSI: 0000000000000000 RDI: 00000000000000e8
> > RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000001
> > R10: 00000000000000e8 R11: 0000000000000000 R12: 0000000000000000
> > R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> > FS: 0000000000000000(0000) GS:ffff8880b9800000(0000)
> > knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000000020000280 CR3: 00000000758bf000 CR4: 0000000000350ef0
> > Call Trace:
> > <TASK>
> > lock_acquire kernel/locking/lockdep.c:5753 [inline]
> > lock_acquire+0x1ae/0x510 kernel/locking/lockdep.c:5718
> > down_write+0x93/0x200 kernel/locking/rwsem.c:1573
> > hugetlb_vma_lock_write mm/hugetlb.c:300 [inline]
> > hugetlb_vma_lock_write+0xae/0x100 mm/hugetlb.c:291
> > __hugetlb_zap_begin+0x1e9/0x2b0 mm/hugetlb.c:5447
> > hugetlb_zap_begin include/linux/hugetlb.h:258 [inline]
> > unmap_vmas+0x2f4/0x470 mm/memory.c:1733
> > exit_mmap+0x1ad/0xa60 mm/mmap.c:3230
> > __mmput+0x12a/0x4d0 kernel/fork.c:1349
> > mmput+0x62/0x70 kernel/fork.c:1371
> > exit_mm kernel/exit.c:567 [inline]
> > do_exit+0x9ad/0x2a20 kernel/exit.c:861
> > __do_sys_exit kernel/exit.c:991 [inline]
> > __se_sys_exit kernel/exit.c:989 [inline]
> > __x64_sys_exit+0x42/0x50 kernel/exit.c:989
> > do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> > do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
> > entry_SYSCALL_64_after_hwframe+0x63/0xcd
> > RIP: 0033:0x7ff2b7a78ab9
> > Code: Unable to access opcode bytes at 0x7ff2b7a78a8f.
> > RSP: 002b:00007fff926ea6b8 EFLAGS: 00000246 ORIG_RAX:
> > 000000000000003c
> > RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007ff2b7a78ab9
> > RDX: 00007ff2b7ab23f3 RSI: 0000000000000000 RDI: 0000000000000000
> > RBP: 000000000000cfda R08: 0000000000000000 R09: 0000000000000006
> > R10: 0000000000000000 R11: 0000000000000246 R12: 00007fff926ea6cc
> > R13: 431bde82d7b634db R14: 0000000000000001 R15: 0000000000000001
> > </TASK>
> > Modules linked in:
> > ---[ end trace 0000000000000000 ]---
> > RIP: 0010:__lock_acquire+0x109/0x5de0 kernel/locking/lockdep.c:5004
> > Code: 45 85 c9 0f 84 cc 0e 00 00 44 8b 05 c1 1e 42 0b 45 85 c0 0f 84
> > be 0d 00 00 48 ba 00 00 00 00 00 fc ff df 4c 89 d1 48 c1 e9 03 <80>
> > 3c 11 00 0f 85 e8 40 00 00 49 81 3a a0 d9 5f 90 0f 84 96 0d 00
> > RSP: 0018:ffffc90003aa7798 EFLAGS: 00010016
> > RAX: ffff88807a0b9dc0 RBX: 1ffff92000754f23 RCX: 000000000000001d
> > RDX: dffffc0000000000 RSI: 0000000000000000 RDI: 00000000000000e8
> > RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000001
> > R10: 00000000000000e8 R11: 0000000000000000 R12: 0000000000000000
> > R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> > FS: 0000000000000000(0000) GS:ffff8880b9800000(0000)
> > knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000000020000280 CR3: 00000000758bf000 CR4: 0000000000350ef0
> > ----------------
> > Code disassembly (best guess):
> > 0: 45 85 c9 test %r9d,%r9d
> > 3: 0f 84 cc 0e 00 00 je 0xed5
> > 9: 44 8b 05 c1 1e 42 0b mov 0xb421ec1(%rip),%r8d #
> > 0xb421ed1
> > 10: 45 85 c0 test %r8d,%r8d
> > 13: 0f 84 be 0d 00 00 je 0xdd7
> > 19: 48 ba 00 00 00 00 00 movabs $0xdffffc0000000000,%rdx
> > 20: fc ff df
> > 23: 4c 89 d1 mov %r10,%rcx
> > 26: 48 c1 e9 03 shr $0x3,%rcx
> > * 2a: 80 3c 11 00 cmpb $0x0,(%rcx,%rdx,1) <--
> > trapping instruction
> > 2e: 0f 85 e8 40 00 00 jne 0x411c
> > 34: 49 81 3a a0 d9 5f 90 cmpq $0xffffffff905fd9a0,(%r10)
> > 3b: 0f .byte 0xf
> > 3c: 84 .byte 0x84
> > 3d: 96 xchg %eax,%esi
> > 3e: 0d .byte 0xd
> >
> >
> > ---
> > This report is generated by a bot. It may contain errors.
> > See https://goo.gl/tpsmEJ for more information about syzbot.
> > syzbot engineers can be reached at syzkaller@googlegroups.com.
> >
> > syzbot will keep track of this issue. See:
> > https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> > For information about bisection process see:
> > https://goo.gl/tpsmEJ#bisection
> >
> > If the bug is already fixed, let syzbot know by replying with:
> > #syz fix: exact-commit-title
> >
> > If you want syzbot to run the reproducer, reply with:
> > #syz test: git://repo/address.git branch-or-commit-hash
> > If you attach or paste a git patch, syzbot will apply it before
> > testing.
> >
> > If you want to overwrite bug's subsystems, reply with:
> > #syz set subsystems: new-subsystem
> > (See the list of subsystem names on the web dashboard)
> >
> > If the bug is a duplicate of another bug, reply with:
> > #syz dup: exact-subject-of-another-report
> >
> > If you want to undo deduplication, reply with:
> > #syz undup
> >
>
> --
> All Rights Reversed.
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/bce5df0508221ab30a1fb121a219034631abedf5.camel%40surriel.com.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write
2023-11-01 14:27 ` Rik van Riel
@ 2023-11-02 12:58 ` Edward Adam Davis
2023-11-02 23:29 ` kernel test robot
` (5 more replies)
2023-11-02 13:46 ` Yin, Fengwei
1 sibling, 6 replies; 17+ messages in thread
From: Edward Adam Davis @ 2023-11-02 12:58 UTC (permalink / raw)
To: riel
Cc: akpm, eadavis, linux-kernel, linux-mm, llvm, mike.kravetz,
muchun.song, nathan, ndesaulniers, syzbot+6ada951e7c0f7bc8a71e,
syzkaller-bugs, trix
When obtaining resv_map from vma, it is necessary to simultaneously determine
the flag HPAGE_RESV_OWNER of vm_private_data.
Only when they are met simultaneously, resv_map is valid.
Reported-and-tested-by: syzbot+6ada951e7c0f7bc8a71e@syzkaller.appspotmail.com
Fixes: bf4916922c60 ("hugetlbfs: extend hugetlb_vma_lock to private VMAs")
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
include/linux/hugetlb.h | 5 ++++-
mm/hugetlb.c | 1 -
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 47d25a5e1933..14babc602f14 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -179,6 +179,7 @@ struct address_space *hugetlb_page_mapping_lock_write(struct page *hpage);
extern int sysctl_hugetlb_shm_group;
extern struct list_head huge_boot_pages;
+static int is_vma_resv_set(struct vm_area_struct *vma, unsigned long flag);
/* arch callbacks */
@@ -1265,9 +1266,11 @@ static inline bool __vma_shareable_lock(struct vm_area_struct *vma)
return (vma->vm_flags & VM_MAYSHARE) && vma->vm_private_data;
}
+#define HPAGE_RESV_OWNER (1UL << 0)
static inline bool __vma_private_lock(struct vm_area_struct *vma)
{
- return (!(vma->vm_flags & VM_MAYSHARE)) && vma->vm_private_data;
+ return (!(vma->vm_flags & VM_MAYSHARE)) && vma->vm_private_data &&
+ is_vma_resv_set(vma, HPAGE_RESV_OWNER);
}
/*
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 1301ba7b2c9a..97ea782dfba6 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1033,7 +1033,6 @@ __weak unsigned long vma_mmu_pagesize(struct vm_area_struct *vma)
* bits of the reservation map pointer, which are always clear due to
* alignment.
*/
-#define HPAGE_RESV_OWNER (1UL << 0)
#define HPAGE_RESV_UNMAPPED (1UL << 1)
#define HPAGE_RESV_MASK (HPAGE_RESV_OWNER | HPAGE_RESV_UNMAPPED)
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write
2023-11-01 14:27 ` Rik van Riel
2023-11-02 12:58 ` Edward Adam Davis
@ 2023-11-02 13:46 ` Yin, Fengwei
1 sibling, 0 replies; 17+ messages in thread
From: Yin, Fengwei @ 2023-11-02 13:46 UTC (permalink / raw)
To: Rik van Riel, Edward Adam Davis, syzbot+6ada951e7c0f7bc8a71e
Cc: akpm, linux-kernel, linux-mm, llvm, mike.kravetz, muchun.song,
nathan, ndesaulniers, syzkaller-bugs, trix
On 1/11/2023 22:27, Rik van Riel wrote:
> On Wed, 2023-11-01 at 14:36 +0800, Edward Adam Davis wrote:
>> When obtaining resv_map from vma, it is necessary to simultaneously
>> determine
>> the flag HPAGE_RESV_OWNER of vm_private_data.
>> Only when they are met simultaneously, resv_map is valid.
>>
>> Reported-and-tested-by:
>> syzbot+6ada951e7c0f7bc8a71e@syzkaller.appspotmail.com
>> Fixes: bf4916922c60 ("hugetlbfs: extend hugetlb_vma_lock to private
>> VMAs")
>> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
>> ---
>> include/linux/hugetlb.h | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
>> index 47d25a5e1933..1a3ec1aee1a3 100644
>> --- a/include/linux/hugetlb.h
>> +++ b/include/linux/hugetlb.h
>> @@ -1265,9 +1265,11 @@ static inline bool __vma_shareable_lock(struct
>> vm_area_struct *vma)
>> return (vma->vm_flags & VM_MAYSHARE) && vma->vm_private_data;
>> }
>>
>> +#define HPAGE_RESV_OWNER (1UL << 0)
>> static inline bool __vma_private_lock(struct vm_area_struct *vma)
>> {
>> - return (!(vma->vm_flags & VM_MAYSHARE)) && vma-
>>> vm_private_data;
>> + return (!(vma->vm_flags & VM_MAYSHARE)) && vma-
>>> vm_private_data &&
>> + ((unsigned long)vma->vm_private_data &
>> HPAGE_RESV_OWNER);
>> }
I am wondering whether this line should be:
return (!(vma->vm_flags & VM_MAYSHARE)) &&
((unsigned long)vma->vm_private_data & HPAGE_RESV_OWNER);
or even:
return (unsigned long)vma->vm_private_data & HPAGE_RESV_OWNER;
because mm/hugetlb.c has:
"We know private mapping must have HPAGE_RESV_OWNER set."
>
> This could be cleaned up a bit by moving the HPAGE_RESV_OWNER
> definition (and its friends) into hugetlb.h, as well as the
> is_vma_resv_set() helper function.
>
> Then __vma_private_lock() can just call is_vma_resv_set(),
> and open coding a duplicate of the same code.
>
> Not having duplicates of the code will make it much harder
> to "miss a spot" with future changes.
>
> I am still struggling to find a place where we might leave
> HPAGE_RESV_OWNER behind on a pointer that is otherwise NULL,
> but if your tests show this fixes the issue, I'm all for it :)
>
Like you said, vma->vm_private_data is cleared during fork().
But I saw following possible code path in child process:
hugetlb_wp()
unmap_ref_private() if alloc_hugetlb_folio fails
unmap_hugepage_range()
__unmap_hugepage_range()
set_vma_resv_flags(vma, HPAGE_RESV_UNMAPPED)
vm_private_data become 0x2 which is none NULL without HPAGE_RESV_OWNER
bit.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write
2023-11-02 12:58 ` Edward Adam Davis
@ 2023-11-02 23:29 ` kernel test robot
2023-11-02 23:29 ` kernel test robot
` (4 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2023-11-02 23:29 UTC (permalink / raw)
To: Edward Adam Davis, riel
Cc: oe-kbuild-all, akpm, eadavis, linux-kernel, linux-mm, llvm,
mike.kravetz, muchun.song, nathan, ndesaulniers,
syzbot+6ada951e7c0f7bc8a71e, syzkaller-bugs, trix
Hi Edward,
kernel test robot noticed the following build errors:
[auto build test ERROR on akpm-mm/mm-everything]
url: https://github.com/intel-lab-lkp/linux/commits/Edward-Adam-Davis/mm-hugetlb-fix-null-ptr-defer-in-hugetlb_vma_lock_write/20231103-003152
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/tencent_164A247533766D667C3D873798968236C409%40qq.com
patch subject: [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write
config: um-defconfig (https://download.01.org/0day-ci/archive/20231103/202311030737.my9iglMI-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231103/202311030737.my9iglMI-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311030737.my9iglMI-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from kernel/fork.c:52:
include/linux/hugetlb.h: In function '__vma_private_lock':
>> include/linux/hugetlb.h:1276:17: error: implicit declaration of function 'is_vma_resv_set' [-Werror=implicit-function-declaration]
1276 | is_vma_resv_set(vma, HPAGE_RESV_OWNER);
| ^~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/is_vma_resv_set +1276 include/linux/hugetlb.h
1271
1272 #define HPAGE_RESV_OWNER (1UL << 0)
1273 static inline bool __vma_private_lock(struct vm_area_struct *vma)
1274 {
1275 return (!(vma->vm_flags & VM_MAYSHARE)) && vma->vm_private_data &&
> 1276 is_vma_resv_set(vma, HPAGE_RESV_OWNER);
1277 }
1278
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write
2023-11-02 12:58 ` Edward Adam Davis
2023-11-02 23:29 ` kernel test robot
@ 2023-11-02 23:29 ` kernel test robot
2023-11-03 0:56 ` Rik van Riel
` (3 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2023-11-02 23:29 UTC (permalink / raw)
To: Edward Adam Davis, riel
Cc: llvm, oe-kbuild-all, akpm, eadavis, linux-kernel, linux-mm,
mike.kravetz, muchun.song, nathan, ndesaulniers,
syzbot+6ada951e7c0f7bc8a71e, syzkaller-bugs, trix
Hi Edward,
kernel test robot noticed the following build errors:
[auto build test ERROR on akpm-mm/mm-everything]
url: https://github.com/intel-lab-lkp/linux/commits/Edward-Adam-Davis/mm-hugetlb-fix-null-ptr-defer-in-hugetlb_vma_lock_write/20231103-003152
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/tencent_164A247533766D667C3D873798968236C409%40qq.com
patch subject: [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write
config: um-allnoconfig (https://download.01.org/0day-ci/archive/20231103/202311030755.DduMG76m-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231103/202311030755.DduMG76m-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311030755.DduMG76m-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from kernel/fork.c:34:
In file included from include/linux/mempolicy.h:15:
In file included from include/linux/pagemap.h:11:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from kernel/fork.c:34:
In file included from include/linux/mempolicy.h:15:
In file included from include/linux/pagemap.h:11:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from kernel/fork.c:34:
In file included from include/linux/mempolicy.h:15:
In file included from include/linux/pagemap.h:11:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
692 | readsb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
700 | readsw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
708 | readsl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
717 | writesb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
726 | writesw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
735 | writesl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
In file included from kernel/fork.c:52:
>> include/linux/hugetlb.h:1276:3: error: call to undeclared function 'is_vma_resv_set'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1276 | is_vma_resv_set(vma, HPAGE_RESV_OWNER);
| ^
12 warnings and 1 error generated.
--
In file included from kernel/sysctl.c:24:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from kernel/sysctl.c:24:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from kernel/sysctl.c:24:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
692 | readsb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
700 | readsw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
708 | readsl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
717 | writesb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
726 | writesw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
735 | writesl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
In file included from kernel/sysctl.c:45:
>> include/linux/hugetlb.h:1276:3: error: call to undeclared function 'is_vma_resv_set'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1276 | is_vma_resv_set(vma, HPAGE_RESV_OWNER);
| ^
In file included from kernel/sysctl.c:53:
In file included from include/linux/nfs_fs.h:31:
In file included from include/linux/sunrpc/auth.h:13:
In file included from include/linux/sunrpc/sched.h:19:
include/linux/sunrpc/xdr.h:782:46: warning: result of comparison of constant 4611686018427387903 with expression of type '__u32' (aka 'unsigned int') is always false [-Wtautological-constant-out-of-range-compare]
782 | if (U32_MAX >= SIZE_MAX / sizeof(*p) && len > SIZE_MAX / sizeof(*p))
| ~~~ ^ ~~~~~~~~~~~~~~~~~~~~~
13 warnings and 1 error generated.
--
In file included from mm/shmem.c:29:
In file included from include/linux/pagemap.h:11:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from mm/shmem.c:29:
In file included from include/linux/pagemap.h:11:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from mm/shmem.c:29:
In file included from include/linux/pagemap.h:11:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
692 | readsb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
700 | readsw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
708 | readsl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
717 | writesb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
726 | writesw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
735 | writesl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
In file included from mm/shmem.c:39:
>> include/linux/hugetlb.h:1276:3: error: call to undeclared function 'is_vma_resv_set'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1276 | is_vma_resv_set(vma, HPAGE_RESV_OWNER);
| ^
In file included from mm/shmem.c:58:
include/linux/mman.h:158:9: warning: division by zero is undefined [-Wdivision-by-zero]
158 | _calc_vm_trans(flags, MAP_SYNC, VM_SYNC ) |
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/mman.h:136:21: note: expanded from macro '_calc_vm_trans'
136 | : ((x) & (bit1)) / ((bit1) / (bit2))))
| ^ ~~~~~~~~~~~~~~~~~
13 warnings and 1 error generated.
vim +/is_vma_resv_set +1276 include/linux/hugetlb.h
1271
1272 #define HPAGE_RESV_OWNER (1UL << 0)
1273 static inline bool __vma_private_lock(struct vm_area_struct *vma)
1274 {
1275 return (!(vma->vm_flags & VM_MAYSHARE)) && vma->vm_private_data &&
> 1276 is_vma_resv_set(vma, HPAGE_RESV_OWNER);
1277 }
1278
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write
2023-11-02 12:58 ` Edward Adam Davis
2023-11-02 23:29 ` kernel test robot
2023-11-02 23:29 ` kernel test robot
@ 2023-11-03 0:56 ` Rik van Riel
2023-11-03 1:26 ` kernel test robot
` (2 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Rik van Riel @ 2023-11-03 0:56 UTC (permalink / raw)
To: Edward Adam Davis
Cc: akpm, linux-kernel, linux-mm, llvm, mike.kravetz, muchun.song,
nathan, ndesaulniers, syzbot+6ada951e7c0f7bc8a71e, syzkaller-bugs,
trix
On Thu, 2023-11-02 at 20:58 +0800, Edward Adam Davis wrote:
>
> +++ b/include/linux/hugetlb.h
>
> +#define HPAGE_RESV_OWNER (1UL << 0)
> static inline bool __vma_private_lock(struct vm_area_struct *vma)
> {
> - return (!(vma->vm_flags & VM_MAYSHARE)) && vma-
> >vm_private_data;
> + return (!(vma->vm_flags & VM_MAYSHARE)) && vma-
> >vm_private_data &&
> + is_vma_resv_set(vma, HPAGE_RESV_OWNER);
> }
>
> /*
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 1301ba7b2c9a..97ea782dfba6 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1033,7 +1033,6 @@ __weak unsigned long vma_mmu_pagesize(struct
> vm_area_struct *vma)
> * bits of the reservation map pointer, which are always clear due
> to
> * alignment.
> */
> -#define HPAGE_RESV_OWNER (1UL << 0)
> #define HPAGE_RESV_UNMAPPED (1UL << 1)
> #define HPAGE_RESV_MASK (HPAGE_RESV_OWNER | HPAGE_RESV_UNMAPPED)
>
Moving just that one define is less than ideal, and the
kernel test robot seems unhappy, too.
It may be cleaner to just move __vma_private_lock into
hugetlb.c, where it has all the dependencies it needs.
It isn't being called from anywhere else, anyway.
Hopefully that will keep the kernel test robot happy, too :)
--
All Rights Reversed.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write
2023-11-02 12:58 ` Edward Adam Davis
` (2 preceding siblings ...)
2023-11-03 0:56 ` Rik van Riel
@ 2023-11-03 1:26 ` kernel test robot
2023-11-03 2:24 ` Mike Kravetz
2023-11-08 3:22 ` Mike Kravetz
5 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2023-11-03 1:26 UTC (permalink / raw)
To: Edward Adam Davis, riel
Cc: oe-kbuild-all, akpm, eadavis, linux-kernel, linux-mm, llvm,
mike.kravetz, muchun.song, nathan, ndesaulniers,
syzbot+6ada951e7c0f7bc8a71e, syzkaller-bugs, trix
Hi Edward,
kernel test robot noticed the following build warnings:
[auto build test WARNING on akpm-mm/mm-everything]
url: https://github.com/intel-lab-lkp/linux/commits/Edward-Adam-Davis/mm-hugetlb-fix-null-ptr-defer-in-hugetlb_vma_lock_write/20231103-003152
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/tencent_164A247533766D667C3D873798968236C409%40qq.com
patch subject: [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write
config: loongarch-randconfig-002-20231103 (https://download.01.org/0day-ci/archive/20231103/202311030935.4AHgxdxU-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231103/202311030935.4AHgxdxU-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311030935.4AHgxdxU-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from include/linux/migrate.h:8,
from include/linux/balloon_compaction.h:41,
from drivers/virtio/virtio_balloon.c:16:
>> include/linux/hugetlb.h:182:12: warning: 'is_vma_resv_set' used but never defined
182 | static int is_vma_resv_set(struct vm_area_struct *vma, unsigned long flag);
| ^~~~~~~~~~~~~~~
vim +/is_vma_resv_set +182 include/linux/hugetlb.h
179
180 extern int sysctl_hugetlb_shm_group;
181 extern struct list_head huge_boot_pages;
> 182 static int is_vma_resv_set(struct vm_area_struct *vma, unsigned long flag);
183
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write
2023-11-02 12:58 ` Edward Adam Davis
` (3 preceding siblings ...)
2023-11-03 1:26 ` kernel test robot
@ 2023-11-03 2:24 ` Mike Kravetz
2023-11-03 2:28 ` Mike Kravetz
2023-11-03 2:37 ` Mike Kravetz
2023-11-08 3:22 ` Mike Kravetz
5 siblings, 2 replies; 17+ messages in thread
From: Mike Kravetz @ 2023-11-03 2:24 UTC (permalink / raw)
To: Edward Adam Davis
Cc: riel, akpm, linux-kernel, linux-mm, llvm, muchun.song, nathan,
ndesaulniers, syzbot+6ada951e7c0f7bc8a71e, syzkaller-bugs, trix
On 11/02/23 20:58, Edward Adam Davis wrote:
> When obtaining resv_map from vma, it is necessary to simultaneously determine
> the flag HPAGE_RESV_OWNER of vm_private_data.
> Only when they are met simultaneously, resv_map is valid.
Thanks for looking into this!
The check for HPAGE_RESV_OWNER does 'work'. However, I believe root
cause is this block of code in __unmap_hugepage_range().
/*
* If a reference page is supplied, it is because a specific
* page is being unmapped, not a range. Ensure the page we
* are about to unmap is the actual page of interest.
*/
if (ref_page) {
if (page != ref_page) {
spin_unlock(ptl);
continue;
}
/*
* Mark the VMA as having unmapped its page so that
* future faults in this VMA will fail rather than
* looking like data was lost
*/
set_vma_resv_flags(vma, HPAGE_RESV_UNMAPPED);
}
In the specific case causing the null-ptr-deref, the resv_map pointer
(vm_private_data) is NULL. So, set_vma_resv_flags() just sets the lower bit.
Because of this, __vma_private_lock returns true.
As mentioned, the check for HPAGE_RESV_OWNER in this patch 'works' because
only the HPAGE_RESV_UNMAPPED bit is set in vm_private_data.
I was thinking a more explicit check for this 'NULL pointer' with lower
bits set could be made in __vma_private_lock. Below is something I put
together. I just open coded the check for 'NULL pointer' instead of
moving a bunch of code to the header file. In addition, I changed the
#defines from HPAGE_* to HUGETLB_* to avoid any confusion as the header
file defines are in the global name space. I thought about also adding
an explicit check for the HPAGE_RESV_OWNER as done in this patch. This
would catch the case where the pointer is not NULL. I do not believe
that is possible in the code today, but might make the check more future
proof.
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 47d25a5e1933..7b472432708e 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -1260,6 +1260,15 @@ bool want_pmd_share(struct vm_area_struct *vma, unsigned long addr);
#define flush_hugetlb_tlb_range(vma, addr, end) flush_tlb_range(vma, addr, end)
#endif
+/*
+ * Flags for MAP_PRIVATE reservations. These are stored in the bottom
+ * bits of the reservation map pointer, which are always clear due to
+ * alignment.
+ */
+#define HUGETLB_RESV_OWNER (1UL << 0)
+#define HUGETLB_RESV_UNMAPPED (1UL << 1)
+#define HUGETLB_RESV_MASK (HUGETLB_RESV_OWNER | HUGETLB_RESV_UNMAPPED)
+
static inline bool __vma_shareable_lock(struct vm_area_struct *vma)
{
return (vma->vm_flags & VM_MAYSHARE) && vma->vm_private_data;
@@ -1267,7 +1276,9 @@ static inline bool __vma_shareable_lock(struct vm_area_struct *vma)
static inline bool __vma_private_lock(struct vm_area_struct *vma)
{
- return (!(vma->vm_flags & VM_MAYSHARE)) && vma->vm_private_data;
+ /* Careful - flags may be set in lower bits of pointer */
+ return (!(vma->vm_flags & VM_MAYSHARE)) &&
+ (unsigned long)vma->vm_private_data & ~HUGETLB_RESV_MASK;
}
/*
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 1301ba7b2c9a..d2215f7647b1 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1028,15 +1028,6 @@ __weak unsigned long vma_mmu_pagesize(struct vm_area_struct *vma)
return vma_kernel_pagesize(vma);
}
-/*
- * Flags for MAP_PRIVATE reservations. These are stored in the bottom
- * bits of the reservation map pointer, which are always clear due to
- * alignment.
- */
-#define HPAGE_RESV_OWNER (1UL << 0)
-#define HPAGE_RESV_UNMAPPED (1UL << 1)
-#define HPAGE_RESV_MASK (HPAGE_RESV_OWNER | HPAGE_RESV_UNMAPPED)
-
/*
* These helpers are used to track how many pages are reserved for
* faults in a MAP_PRIVATE mapping. Only the process that called mmap()
@@ -1162,7 +1153,7 @@ static struct resv_map *vma_resv_map(struct vm_area_struct *vma)
} else {
return (struct resv_map *)(get_vma_private_data(vma) &
- ~HPAGE_RESV_MASK);
+ ~HUGETLB_RESV_MASK);
}
}
@@ -1236,7 +1227,7 @@ void clear_vma_resv_huge_pages(struct vm_area_struct *vma)
*/
struct resv_map *reservations = vma_resv_map(vma);
- if (reservations && is_vma_resv_set(vma, HPAGE_RESV_OWNER)) {
+ if (reservations && is_vma_resv_set(vma, HUGETLB_RESV_OWNER)) {
resv_map_put_hugetlb_cgroup_uncharge_info(reservations);
kref_put(&reservations->refs, resv_map_release);
}
@@ -1282,7 +1273,7 @@ static bool vma_has_reserves(struct vm_area_struct *vma, long chg)
* Only the process that called mmap() has reserves for
* private mappings.
*/
- if (is_vma_resv_set(vma, HPAGE_RESV_OWNER)) {
+ if (is_vma_resv_set(vma, HUGETLB_RESV_OWNER)) {
/*
* Like the shared case above, a hole punch or truncate
* could have been performed on the private mapping.
@@ -2763,7 +2754,7 @@ static long __vma_reservation_common(struct hstate *h,
if (vma->vm_flags & VM_MAYSHARE || mode == VMA_DEL_RESV)
return ret;
/*
- * We know private mapping must have HPAGE_RESV_OWNER set.
+ * We know private mapping must have HUGETLB_RESV_OWNER set.
*
* In most cases, reserves always exist for private mappings.
* However, a file associated with mapping could have been
@@ -4833,7 +4824,7 @@ static void hugetlb_vm_op_open(struct vm_area_struct *vma)
struct resv_map *resv = vma_resv_map(vma);
/*
- * HPAGE_RESV_OWNER indicates a private mapping.
+ * HUGETLB_RESV_OWNER indicates a private mapping.
* This new VMA should share its siblings reservation map if present.
* The VMA will only ever have a valid reservation map pointer where
* it is being copied for another still existing VMA. As that VMA
@@ -4841,7 +4832,7 @@ static void hugetlb_vm_op_open(struct vm_area_struct *vma)
* after this open call completes. It is therefore safe to take a
* new reference here without additional locking.
*/
- if (resv && is_vma_resv_set(vma, HPAGE_RESV_OWNER)) {
+ if (resv && is_vma_resv_set(vma, HUGETLB_RESV_OWNER)) {
resv_map_dup_hugetlb_cgroup_uncharge_info(resv);
kref_get(&resv->refs);
}
@@ -4877,7 +4868,7 @@ static void hugetlb_vm_op_close(struct vm_area_struct *vma)
hugetlb_vma_lock_free(vma);
resv = vma_resv_map(vma);
- if (!resv || !is_vma_resv_set(vma, HPAGE_RESV_OWNER))
+ if (!resv || !is_vma_resv_set(vma, HUGETLB_RESV_OWNER))
return;
start = vma_hugecache_offset(h, vma, vma->vm_start);
@@ -5394,7 +5385,7 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,
* future faults in this VMA will fail rather than
* looking like data was lost
*/
- set_vma_resv_flags(vma, HPAGE_RESV_UNMAPPED);
+ set_vma_resv_flags(vma, HUGETLB_RESV_UNMAPPED);
}
pte = huge_ptep_get_and_clear(mm, address, ptep);
@@ -5544,7 +5535,7 @@ static void unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,
* could insert a zeroed page instead of the data existing
* from the time of fork. This would look like data corruption
*/
- if (!is_vma_resv_set(iter_vma, HPAGE_RESV_OWNER))
+ if (!is_vma_resv_set(iter_vma, HUGETLB_RESV_OWNER))
unmap_hugepage_range(iter_vma, address,
address + huge_page_size(h), page, 0);
}
@@ -5625,7 +5616,7 @@ static vm_fault_t hugetlb_wp(struct mm_struct *mm, struct vm_area_struct *vma,
* at the time of fork() could consume its reserves on COW instead
* of the full address range.
*/
- if (is_vma_resv_set(vma, HPAGE_RESV_OWNER) &&
+ if (is_vma_resv_set(vma, HUGETLB_RESV_OWNER) &&
old_folio != pagecache_folio)
outside_reserve = 1;
@@ -5865,7 +5856,7 @@ static vm_fault_t hugetlb_no_page(struct mm_struct *mm,
* COW/unsharing. Warn that such a situation has occurred as it may not
* be obvious.
*/
- if (is_vma_resv_set(vma, HPAGE_RESV_UNMAPPED)) {
+ if (is_vma_resv_set(vma, HUGETLB_RESV_UNMAPPED)) {
pr_warn_ratelimited("PID %d killed due to inadequate hugepage pool\n",
current->pid);
goto out;
@@ -6756,7 +6747,7 @@ bool hugetlb_reserve_pages(struct inode *inode,
chg = to - from;
set_vma_resv_map(vma, resv_map);
- set_vma_resv_flags(vma, HPAGE_RESV_OWNER);
+ set_vma_resv_flags(vma, HUGETLB_RESV_OWNER);
}
if (chg < 0)
@@ -6853,7 +6844,7 @@ bool hugetlb_reserve_pages(struct inode *inode,
*/
if (chg >= 0 && add < 0)
region_abort(resv_map, from, to, regions_needed);
- if (vma && is_vma_resv_set(vma, HPAGE_RESV_OWNER)) {
+ if (vma && is_vma_resv_set(vma, HUGETLB_RESV_OWNER)) {
kref_put(&resv_map->refs, resv_map_release);
set_vma_resv_map(vma, NULL);
}
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write
2023-11-03 2:24 ` Mike Kravetz
@ 2023-11-03 2:28 ` Mike Kravetz
2023-11-03 2:37 ` Mike Kravetz
1 sibling, 0 replies; 17+ messages in thread
From: Mike Kravetz @ 2023-11-03 2:28 UTC (permalink / raw)
To: Edward Adam Davis, Yin, Fengwei
Cc: riel, akpm, linux-kernel, linux-mm, llvm, muchun.song, nathan,
ndesaulniers, syzbot+6ada951e7c0f7bc8a71e, syzkaller-bugs, trix
On 11/02/23 19:24, Mike Kravetz wrote:
> On 11/02/23 20:58, Edward Adam Davis wrote:
> > When obtaining resv_map from vma, it is necessary to simultaneously determine
> > the flag HPAGE_RESV_OWNER of vm_private_data.
> > Only when they are met simultaneously, resv_map is valid.
>
> Thanks for looking into this!
>
> The check for HPAGE_RESV_OWNER does 'work'. However, I believe root
> cause is this block of code in __unmap_hugepage_range().
>
> /*
> * If a reference page is supplied, it is because a specific
> * page is being unmapped, not a range. Ensure the page we
> * are about to unmap is the actual page of interest.
> */
> if (ref_page) {
> if (page != ref_page) {
> spin_unlock(ptl);
> continue;
> }
> /*
> * Mark the VMA as having unmapped its page so that
> * future faults in this VMA will fail rather than
> * looking like data was lost
> */
> set_vma_resv_flags(vma, HPAGE_RESV_UNMAPPED);
> }
>
> In the specific case causing the null-ptr-deref, the resv_map pointer
> (vm_private_data) is NULL. So, set_vma_resv_flags() just sets the lower bit.
> Because of this, __vma_private_lock returns true.
Ah!
I see Yin, Fengwei already discovered this code path.
--
Mike Kravetz
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write
2023-11-03 2:24 ` Mike Kravetz
2023-11-03 2:28 ` Mike Kravetz
@ 2023-11-03 2:37 ` Mike Kravetz
2023-11-03 3:15 ` Rik van Riel
1 sibling, 1 reply; 17+ messages in thread
From: Mike Kravetz @ 2023-11-03 2:37 UTC (permalink / raw)
To: Rik van Riel
Cc: Edward Adam Davis, akpm, linux-kernel, linux-mm, llvm,
muchun.song, nathan, ndesaulniers, syzbot+6ada951e7c0f7bc8a71e,
syzkaller-bugs, trix
On 11/02/23 19:24, Mike Kravetz wrote:
>
> In the specific case causing the null-ptr-deref, the resv_map pointer
> (vm_private_data) is NULL.
Hi Rik,
In commit bf4916922c60 hugetlbfs: extend hugetlb_vma_lock to private VMAs,
it correctly says:
Extend the locking scheme used to protect shared hugetlb mappings from
truncate vs page fault races, in order to protect private hugetlb mappings
(with resv_map) against MADV_DONTNEED.
That qualification '(with resv_map)' caught my attention originally, and
I thought about it again while looking into this. We now cover the common
cases, but there are still quite a few cases where resv_map is NULL for
private mappings. In such cases, the race between MADV_DONTNEED and page
fault still exists. Is that a concern?
With a bit more work we 'could' make sure every hugetlb vma has a lock
to participate in this scheme.
Any thhoughts?
--
Mike Kravetz
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write
2023-11-03 2:37 ` Mike Kravetz
@ 2023-11-03 3:15 ` Rik van Riel
2023-11-03 4:31 ` Mike Kravetz
0 siblings, 1 reply; 17+ messages in thread
From: Rik van Riel @ 2023-11-03 3:15 UTC (permalink / raw)
To: Mike Kravetz
Cc: Edward Adam Davis, akpm, linux-kernel, linux-mm, llvm,
muchun.song, nathan, ndesaulniers, syzbot+6ada951e7c0f7bc8a71e,
syzkaller-bugs, trix
On Thu, 2023-11-02 at 19:37 -0700, Mike Kravetz wrote:
> On 11/02/23 19:24, Mike Kravetz wrote:
> >
> > In the specific case causing the null-ptr-deref, the resv_map
> > pointer
> > (vm_private_data) is NULL.
>
> Hi Rik,
>
> In commit bf4916922c60 hugetlbfs: extend hugetlb_vma_lock to private
> VMAs,
> it correctly says:
>
> Extend the locking scheme used to protect shared hugetlb mappings
> from
> truncate vs page fault races, in order to protect private hugetlb
> mappings
> (with resv_map) against MADV_DONTNEED.
>
> That qualification '(with resv_map)' caught my attention originally,
> and
> I thought about it again while looking into this. We now cover the
> common
> cases, but there are still quite a few cases where resv_map is NULL
> for
> private mappings. In such cases, the race between MADV_DONTNEED and
> page
> fault still exists. Is that a concern?
Honestly, I'm not sure. In hugetlb_dup_vma_private, which is
called at fork time, we have this comment:
* - For MAP_PRIVATE mappings, this is the reserve map which
does
* not apply to children. Faults generated by the children
are
* not guaranteed to succeed, even if read-only.
That suggests we already have no guarantee of faults
succeeding after fork.
>
> With a bit more work we 'could' make sure every hugetlb vma has a
> lock
> to participate in this scheme.
>
> Any thhoughts?
We can certainly close the race between MADV_DONTNEED
and page faults for MAP_PRIVATE mappings in child processes,
but that does not guarantee that we actually have hugetlb
pages for those processes.
In short, I'm not sure :)
--
All Rights Reversed.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write
2023-11-03 3:15 ` Rik van Riel
@ 2023-11-03 4:31 ` Mike Kravetz
0 siblings, 0 replies; 17+ messages in thread
From: Mike Kravetz @ 2023-11-03 4:31 UTC (permalink / raw)
To: Rik van Riel
Cc: Edward Adam Davis, akpm, linux-kernel, linux-mm, llvm,
muchun.song, nathan, ndesaulniers, syzbot+6ada951e7c0f7bc8a71e,
syzkaller-bugs, trix
On 11/02/23 23:15, Rik van Riel wrote:
> On Thu, 2023-11-02 at 19:37 -0700, Mike Kravetz wrote:
> > On 11/02/23 19:24, Mike Kravetz wrote:
> > That qualification '(with resv_map)' caught my attention originally,
> > and
> > I thought about it again while looking into this. We now cover the
> > common
> > cases, but there are still quite a few cases where resv_map is NULL
> > for
> > private mappings. In such cases, the race between MADV_DONTNEED and
> > page
> > fault still exists. Is that a concern?
>
> Honestly, I'm not sure. In hugetlb_dup_vma_private, which is
> called at fork time, we have this comment:
>
> * - For MAP_PRIVATE mappings, this is the reserve map which
> does
> * not apply to children. Faults generated by the children
> are
> * not guaranteed to succeed, even if read-only.
>
> That suggests we already have no guarantee of faults
> succeeding after fork.
Right!
>
> >
> > With a bit more work we 'could' make sure every hugetlb vma has a
> > lock
> > to participate in this scheme.
> >
> > Any thhoughts?
>
> We can certainly close the race between MADV_DONTNEED
> and page faults for MAP_PRIVATE mappings in child processes,
> but that does not guarantee that we actually have hugetlb
> pages for those processes.
>
> In short, I'm not sure :)
I sort of remember something Dave Hansen added years ago to help a customer
allocating LOTs of hugetlb pages dynamically. I seem to recall that this
was to get better numa locality. As a result, they did not use reservations.
I guess it sticks with me because it was/is a real example of a customer
choosing NOT to use reservations.
I don't have any evidence that this is common. My thought is to leave
it as is until someone complains.
--
Mike Kravetz
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mm/hugetlb: fix null ptr defer in hugetlb_vma_lock_write
2023-11-02 12:58 ` Edward Adam Davis
` (4 preceding siblings ...)
2023-11-03 2:24 ` Mike Kravetz
@ 2023-11-08 3:22 ` Mike Kravetz
5 siblings, 0 replies; 17+ messages in thread
From: Mike Kravetz @ 2023-11-08 3:22 UTC (permalink / raw)
To: Edward Adam Davis
Cc: riel, akpm, linux-kernel, linux-mm, llvm, muchun.song, nathan,
ndesaulniers, syzbot+6ada951e7c0f7bc8a71e, syzkaller-bugs, trix
Hello Edward,
Do you plan on following up with a new version of the patch?
My suggestion would be:
- Move __vma_private_lock into hugetlb.c as suggeted by Rik.
- Change __vma_private_lock to check for NULL resv_map even if low bit
flags are set as well as HPAGE_RESV_OWNER.
--
Mike Kravetz
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-11-08 3:23 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-29 9:27 [syzbot] [mm?] general protection fault in hugetlb_vma_lock_write syzbot
2023-10-31 18:38 ` Rik van Riel
2023-11-02 12:15 ` Aleksandr Nogikh
2023-11-01 6:36 ` [PATCH] mm/hugetlb: fix null ptr defer " Edward Adam Davis
2023-11-01 14:27 ` Rik van Riel
2023-11-02 12:58 ` Edward Adam Davis
2023-11-02 23:29 ` kernel test robot
2023-11-02 23:29 ` kernel test robot
2023-11-03 0:56 ` Rik van Riel
2023-11-03 1:26 ` kernel test robot
2023-11-03 2:24 ` Mike Kravetz
2023-11-03 2:28 ` Mike Kravetz
2023-11-03 2:37 ` Mike Kravetz
2023-11-03 3:15 ` Rik van Riel
2023-11-03 4:31 ` Mike Kravetz
2023-11-08 3:22 ` Mike Kravetz
2023-11-02 13:46 ` Yin, Fengwei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).