Bug 36504
| Summary: | [kernel] [patch] crash/panic vm_object_allocate under file system code | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Dorr H. Clark <dclark> | ||||||
| Component: | kern | Assignee: | Alan Cox <alc> | ||||||
| Status: | Closed FIXED | ||||||||
| Severity: | Affects Only Me | ||||||||
| Priority: | Normal | ||||||||
| Version: | Unspecified | ||||||||
| Hardware: | Any | ||||||||
| OS: | Any | ||||||||
| Attachments: |
|
||||||||
Responsible Changed From-To: freebsd-bugs->dillon I'll look at this later on tuesday. It makes sense to have a defensive check but I need more information on the failure case itself. i.e. is it when the zalloc area runs out of VM or is it a temporary, recoverable no-free-pages condition? A better diff, and a better fix in vfs_default.c:
--- orig/vnode_pager.c Wed Mar 27 00:40:08 2002
+++ other/vnode_pager.c Tue Apr 2 13:49:43 2002
@@ -143,13 +143,15 @@
* And an object of the appropriate size
*/
object = vm_object_allocate(OBJT_VNODE,
OFF_TO_IDX(round_page(size)));
- object->flags = 0;
+ if (object) {
+ object->flags = 0;
- object->un_pager.vnp.vnp_size = size;
-
- object->handle = handle;
- vp->v_object = object;
- vp->v_usecount++;
+ object->un_pager.vnp.vnp_size = size;
+
+ object->handle = handle;
+ vp->v_object = object;
+ vp->v_usecount++;
+ }
} else {
object->ref_count++;
vp->v_usecount++;
--- orig/vfs_default.c Wed Mar 27 00:41:10 2002
+++ other/vfs_default.c Tue Apr 2 13:49:29 2002
@@ -526,9 +526,8 @@
* cause any problems (yet).
*/
object = vnode_pager_alloc(vp,
IDX_TO_OFF(INT_MAX),
0, 0);
- } else {
- goto retn;
}
+ if (object == NULL) goto retn;
/*
* Dereference the reference we just created. This
assumes
* that the object is associated with the vp.
The original fix was incomplete, because vm_object_allocate
itself is not null tolerant. Sorry! I was misinformed
by an incomplete stack trace. Here is the the third file
completing the fix.
--- /usr/src/sys/vm/vm_object.c Fri Aug 4 15:31:11 2000
+++ ./vm_object.c Tue Apr 9 20:44:39 2002
@@ -223,9 +223,8 @@
{
vm_object_t result;
- result = (vm_object_t) zalloc(obj_zone);
-
- _vm_object_allocate(type, size, result);
+ if ((result = (vm_object_t) zalloc(obj_zone)))
+ _vm_object_allocate(type, size, result);
return (result);
}
An audit of other references to vm_object_allocate reveals
that about half are null tolerant and half are not.
This set of changes cleans up most of the other unprotected
references.
-dhc
--- /usr/src/sys/vm/device_pager.c Wed Aug 2 14:54:37 2000
+++ device_pager.c Tue Apr 30 12:44:59 2002
@@ -147,9 +147,11 @@
*/
object = vm_object_allocate(OBJT_DEVICE,
OFF_TO_IDX(foff + size));
- object->handle = handle;
- TAILQ_INIT(&object->un_pager.devp.devp_pglist);
- TAILQ_INSERT_TAIL(&dev_pager_object_list, object,
pager_object_list);
+ if (object) {
+ object->handle = handle;
+ TAILQ_INIT(&object->un_pager.devp.devp_pglist);
+ TAILQ_INSERT_TAIL(&dev_pager_object_list,
object,
pager_object_list);
+ }
} else {
/*
* Gain a reference to the object.
--- /usr/src/sys/vm/phys_pager.c Sat Dec 16 18:05:41 2000
+++ phys_pager.c Tue Apr 30 12:44:59 2002
@@ -85,9 +85,11 @@
*/
object = vm_object_allocate(OBJT_PHYS,
OFF_TO_IDX(foff + size));
- object->handle = handle;
- TAILQ_INSERT_TAIL(&phys_pager_object_list,
object,
- pager_object_list);
+ if (object) {
+ object->handle = handle;
+
TAILQ_INSERT_TAIL(&phys_pager_object_list,
+ object, pager_object_list);
+ }
} else {
/*
* Gain a reference to the object.
--- /usr/src/sys/vm/swap_pager.c Fri Aug 24 15:54:33 2001
+++ swap_pager.c Tue Apr 30 13:13:23 2002
@@ -377,9 +409,10 @@
} else {
object = vm_object_allocate(OBJT_DEFAULT,
OFF_TO_IDX(offset + PAGE_MASK + size));
- object->handle = handle;
-
- swp_pager_meta_build(object, 0, SWAPBLK_NONE);
+ if (object) {
+ object->handle = handle;
+ swp_pager_meta_build(object, 0,
SWAPBLK_NONE);
+ }
}
if (sw_alloc_interlock < 0)
@@ -389,8 +422,7 @@
} else {
object = vm_object_allocate(OBJT_DEFAULT,
OFF_TO_IDX(offset + PAGE_MASK + size));
-
- swp_pager_meta_build(object, 0, SWAPBLK_NONE);
+ if (object) swp_pager_meta_build(object, 0,
SWAPBLK_NONE);
}
return (object);
Running with the changes listed for the last four months, problem is no longer manifest, fix seems to be working. -dhc Responsible Changed From-To: dillon->freebsd-bugs Back to the free pool. State Changed From-To: open->feedback This may no longer be an issue. Responsible Changed From-To: freebsd-bugs->alc This may no longer be an issue. State Changed From-To: feedback->closed Ever since the introduction of UMA, we have used M_WAITOK to allocate vm objects. So, it is now impossible for vm_object_allocate() to return NULL. ----- Forwarded message from "Dorr H. Clark" <dclark@engr.scu.edu> ----- Hi Kip- The fix I posted is specific to the FreeBSD 4.x kernel VM implementation, it's no longer relevant to "CURRENT". Thanks, -dhc |
The problem is that unlike other parts of the kernel, vnode_pager_alloc assumes that the call to vm_object_allocate will succeed. If the allocation fails, the kernel crashes immediately. I have put in a defensive check for this, and also changed the error handling in the calling routine to account for the allocation failure. Fix: My fix touches two files, vm/vnode_pager.c and kern/vfs_default.c, I chose to minimize code changes but I think vfs_default.c could be favorably restructured a little in this area. Sorry for the grody diff but I'm on HPUX here. First file change vm/vnode_pager.c: Second file change kern/vfs_default.c: *************** ! (!(object = vnode_pager_alloc(vp, vat.va_size, 0, 0)))) goto retn; } else if (devsw(vp->v_rdev) != NULL) { /* * This simply allocates the biggest object possible --- 518,526 ---- retry: if ((object = vp->v_object) == NULL) { if (vp->v_type == VREG || vp->v_type == VDIR) { ! if ((error = VOP_GETATTR(vp, &vat, cred, p)) != 0) goto retn; + object = vnode_pager_alloc(vp, vat.va_size, 0, 0); } else if (devsw(vp->v_rdev) != NULL) { /* * This simply allocates the biggest object possible *************** * cause any problems (yet). */ ! if (!(object = vnode_pager_alloc(vp, IDX_TO_OFF(INT_MAX ), 0, 0))) ! goto retn; } else { goto retn; } --- 527,533 ---- * for a disk vnode. This should be fixed, but doesn 't * cause any problems (yet). */ ! object = vnode_pager_alloc(vp, IDX_TO_OFF(INT_MAX), 0 , 0); } else { goto retn; } ***************--qxfEZ06fuR8v9b6E4EH5cdAAd0WLzp4hHcRYbDRg91q0tkiL Content-Type: text/plain; name="file.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="file.diff" *** 143,149 **** * And an object of the appropriate size */ object = vm_object_allocate(OBJT_VNODE, OFF_TO_IDX(round_page(s ize))); + if (object) { object->flags = 0; object->un_pager.vnp.vnp_size = size; --- 143,148 ---- *************** *** 151,157 **** object->handle = handle; vp->v_object = object; vp->v_usecount++; + } } else { object->ref_count++; vp->v_usecount++; --- 150,155 ---- How-To-Repeat: hard to do - it's intermittent under heavy load w/almost no memory