Project

General

Profile

Actions

Bug #13944

closed

librados assumes malloc(0) returns valid pointer

Added by Dan Mick over 8 years ago. Updated over 8 years ago.

Status:
Resolved
Priority:
High
Assignee:
Category:
librados
Target version:
-
% Done:

0%

Source:
Development
Tags:
Backport:
Regression:
No
Severity:
2 - major
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Description

librados relies on malloc(0) returning a valid pointer; this is not portable, and even Linux's manpage says "If size is 0, then malloc() returns either NULL, or a unique pointer value that can later be successfully passed to free()."

The particular instance I found was in rados_getxattrs_next(), where AIX failed CReadOpsTest.GetXattrs because of it, but it probably wouldn't hurt to look over other usages of malloc() proactively.

Actions #1

Updated by Dan Mick over 8 years ago

  • Description updated (diff)
Actions #2

Updated by Josh Durgin over 8 years ago

we should really just use new and delete instead

Actions #3

Updated by Dan Mick over 8 years ago

Maybe new/delete are right.

Investigating effects of changing to return NULL for val when len == 0:

ctypes.string_at(), used in rados.py XattrIterator, returns a zero-length string when passed a NULL pointer and zero length.

the tests LibRadosIo{EC}, XattrIter in test/librados/io.cc may need extra conditionals to avoid the memcmp if val is NULL (behavior is not defined for invalid pointers)… The currently-defined attrs won't cause the condition, but if someone changes that set, it may.

src/tools/scratchtool.c needs protection in do_rados_getxattrs()

src/tracing/librados.tp looks like ceph_ctf_sequence handles NULLs

Actions #4

Updated by Dan Mick over 8 years ago

libradosstriper's test program also needs the check.

It looks like a possibly-existing second bug is that rados.py leaks the memory for the returned value(s); not certain whether ctypes.string_at() copies or creates a reference to the underlying buffer. The test programs definitely leak, but no one cares.

Actions #5

Updated by Sage Weil over 8 years ago

  • Priority changed from Normal to High
Actions #6

Updated by Dan Mick over 8 years ago

  • Status changed from In Progress to Fix Under Review

The symptom for users of librados would be that rados_getxattr_next fails with ENOMEM on attributes of length 0, so this isn't a backward-compatibility issue.

Actions #7

Updated by Sage Weil over 8 years ago

  • Status changed from Fix Under Review to Resolved

not sure this is worth backporting.

Actions

Also available in: Atom PDF