pvn_getpages may assert in valid scenarios
The assertion tries to ensure that the page list is large enough for the result. However, it fails to take into consideration that the page list is optional - and that it that case the page list size will be less than the expected length.
> ::status debugging crash dump vmcore.0 (64-bit) from osiris operating system: 5.11 pvn-getpages (i86pc) image uuid: 52104cf6-9217-ce01-ef62-b2599905c0d0 panic message: assertion failed: plsz >= len, file: ../../common/vm/vm_pvn.c, line: 1118 dump content: kernel pages only > $c vpanic() 0xfffffffffbdf23c8() pvn_getpages+0x1f5(fffffffff79e9460, ffffff05b3738f00, 0, 1000, 0, 0) tmp_getpage+0x1a8(ffffff05b3738f00, 0, 1000, 0, 0, 0) fop_getpage+0x7e(ffffff05b3738f00, 0, 1000, 0, 0, 0) segvn_faulta+0x17c(ffffff053e44d780, 8050000) as_faulta+0x143(ffffff0b54055800, 8050000, 23f8) memcntl+0x53d(8050000, 23f8, 4, 3, 0, 0) sys_syscall32+0x1f7()
The 5th and 6th args to tmp_getpage are both 0s - IOW, pl NULL, plsz 0. This is valid.
Updated by Gordon Ross almost 7 years ago
This could also be considered a bug in the caller, depending on what we believe the expected operation of pvn_getpages should be. Is that documented anywhere? Or how was it decided what should be considered correct behavior for that function?
Updated by Josef Sipek almost 7 years ago
Sadly, there is no real documentation for the vm/vfs/vn code. Since pvn_getpages is called by just about every filesystem, I think it's better to handle this case in pvn_getpages instead of making every caller deal with it.
pl being NULL and plsz being 0 is valid and it can happen if a user application calls madvise(3c). In the above stack trace, the user process called:
madvise(0x8050000, 0x23f8, MADV_WILLNEED);
This ended up in the filesystem as a single-page fop_getpage, with pl=NULL and plsz=0. This doesn't cause problems today because no filesystems call pvn_getpages with lengths <= PAGESIZE, so we just squeak by. My other pvn_getpages changes uncovered this.
Updated by Electric Monk over 6 years ago
- Status changed from New to Closed
- % Done changed from 80 to 100
commit ca411232542ddf24874b545c5f0ec23b6b45f5f2 Author: Josef 'Jeff' Sipek <firstname.lastname@example.org> Date: 2015-02-26T19:30:03.000Z 5384 pvn_getpages may assert in valid scenarios Reviewed by: Marcel Telka <email@example.com> Approved by: Dan McDonald <firstname.lastname@example.org>