Bug #5384
closed
pvn_getpages may assert in valid scenarios
100%
Description
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 9 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 9 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 almost 9 years ago
- Status changed from New to Closed
- % Done changed from 80 to 100
git commit ca411232542ddf24874b545c5f0ec23b6b45f5f2
commit ca411232542ddf24874b545c5f0ec23b6b45f5f2
Author: Josef 'Jeff' Sipek <josef.sipek@nexenta.com>
Date: 2015-02-26T19:30:03.000Z
5384 pvn_getpages may assert in valid scenarios
Reviewed by: Marcel Telka <marcel@telka.sk>
Approved by: Dan McDonald <danmcd@omniti.com>