Project

General

Profile

Bug #7363

smbios may improperly free data

Added by Robert Mustacchi about 3 years ago. Updated about 3 years ago.

Status:
New
Priority:
Normal
Category:
kernel
Start date:
2016-09-07
Due date:
% Done:

80%

Estimated time:
Difficulty:
Medium
Tags:

Description

While reviewing code for 7360, I discovered what may be a sublte and rare bug in the way that smbios frees its data.

When allocating a handle in smbios_open(), it first allocates memory for the sh_structs member and then the sh_hash member. After this it checks both. If either are NULL, then it will close the handle via the open error. This code path will then close the handle since it's not NULL. At which point, it will unconditionally call smb_free on both sh_structs and sh_hash, both of which will have a non-zero length.

For the library version, this isn't a problem, since it's just a thin wrapper around malloc/free. However, for the kernel it's a different story. The kernel version in uts/common/os/smb_subr.c will just call kmem_free, passing the buffer and size. However, this is a problem. While kmem_free(NULL, 0) is always safe, performing kmem_free(NULL, size) is not safe. kmem_free will assume that the buffer is valid. In this case, the logic in smb_free() should check for being given a NULL pointer and not proceed with calling kmem_free, since it knows that this is a special case.

History

#1

Updated by Robert Mustacchi about 3 years ago

Patrick pointed out that this is pretty minor as all of the current smb allocations use KM_SLEEP so we're unlikely to get down this path. But it seems prudent here to allay any future changes that miss this or other buggy behavior.

Also available in: Atom PDF