Project

General

Profile

Bug #5384

pvn_getpages may assert in valid scenarios

Added by Josef Sipek over 5 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
kernel
Start date:
2014-12-03
Due date:
% Done:

100%

Estimated time:
Difficulty:
Bite-size
Tags:
needs-triage
Gerrit CR:

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.

History

#1

Updated by Gordon Ross over 5 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?

#2

Updated by Josef Sipek over 5 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.

#3

Updated by Electric Monk over 5 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>

Also available in: Atom PDF