Project

General

Profile

Bug #13944

librados assumes malloc(0) returns valid pointer

Added by Dan Mick about 3 years ago. Updated about 3 years ago.

Status:
Resolved
Priority:
High
Assignee:
Category:
librados
Target version:
-
Start date:
12/01/2015
Due date:
% Done:

0%

Source:
Development
Tags:
Backport:
Regression:
No
Severity:
2 - major
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:

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.

Associated revisions

Revision 4ec0def9 (diff)
Added by Dan Mick about 3 years ago

librados.cc: rados_getxattrs_next: don't try to use malloc(0)

If an xattr is 0 length, don't try to malloc a buffer for the value;
leave it NULL. (Linux doesn't promise malloc(0) returns a pointer,
and other implementations, like AIX, definitely return NULL.)

Usage changes are in following commits.

Fixes: #13944
Signed-off-by: Dan Mick <>

Revision 708ec2b8 (diff)
Added by Dan Mick about 3 years ago

tests and tools/scratchtool: Don't attempt to use NULL xattr

Prior commit changes rados{striper}_getxattrs_next to be able to return
NULL in the 'val' param. Handle that.

Fixes: #13944
Signed-off-by: Dan Mick <>

History

#1 Updated by Dan Mick about 3 years ago

  • Description updated (diff)

#2 Updated by Josh Durgin about 3 years ago

we should really just use new and delete instead

#3 Updated by Dan Mick about 3 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

#4 Updated by Dan Mick about 3 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.

#5 Updated by Sage Weil about 3 years ago

  • Priority changed from Normal to High

#6 Updated by Dan Mick about 3 years ago

  • Status changed from In Progress to Need 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.

#7 Updated by Sage Weil about 3 years ago

  • Status changed from Need Review to Resolved

not sure this is worth backporting.

Also available in: Atom PDF