[haiku-bugs] Re: [Haiku] #6312: slab: protection from wrong freed objects
- From: "lucian" <trac@xxxxxxxxxxxx>
- Date: Tue, 13 Jul 2010 23:15:31 -0000
#6312: slab: protection from wrong freed objects
----------------------------+-----------------------------------------------
Reporter: lucian | Owner: axeld
Type: enhancement | Status: new
Priority: normal | Milestone: R1
Component: System/Kernel | Version: R1/Development
Resolution: | Keywords:
Blocked By: | Has a Patch: 1
Platform: All | Blocking:
----------------------------+-----------------------------------------------
Comment (by lucian):
Are you sure the second check is not correct?
{{{
intptr_t objectOffset = data - source->offset - (uint8*) source->pages;
if (objectOffset % object_size != 0) { panic... }
}}}
I do remove the slab::offset before checking: {{{data - source->offset}}}.
I took as guide the way objects are created in InitSlab:
{{{
uint8* data = ((uint8*)pages) + slab->offset;
for (size_t i = 0; i < slab->size; i++) {
status = constructor(cookie, data);
data += object_size;
}
}}}
The lower bounds check can be tighter (should be {{{data < source->pages +
source->offset}}} I'll address that in a second version, but as far as I
understand the code there's no off-by-one error (the check uses ">=", not
">").
I wasn't aware one could return **after** a {{{panic()}}}.
I'm not as sure as you are about the KDEBUG >= 1 checks. Trashing a slab's
->free list can easily lead to data corruption or data loss. As a user
(not only as a developer) I'd prefer to reboot than have data silently
corrupted (think of a malloc() from a file system that receives a pointer
to an object that's being written by someone else in parallel corrupting
data in transit or before being sent to the disk).
I'll put them in because you requested them, but I still disagree with
you.
I've rewritten the patch.
--
Ticket URL: <http://dev.haiku-os.org/ticket/6312#comment:3>
Haiku <http://dev.haiku-os.org>
Haiku - the operating system.
Other related posts:
