Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

common/obj_bencher.cc: fix verification crashing when there's no objects #5853

Merged
2 commits merged into from Sep 30, 2015

Conversation

Projects
None yet
3 participants
@branch-predictor
Copy link
Member

branch-predictor commented Sep 9, 2015

When specified pool is empty or doesn't contain valid bench objects, rados
can crash because it attempts to memcmp through NULL pointer. Added check
for valid object size (which also fixes the case when objects are there, but
they're truncated), and made object_size being size_t instead of int.

Also, fix lock issue under similar conditions in seq_read_bench().

Signed-off-by: Piotr Dałek piotr.dalek@ts.fujitsu.com

@ghost ghost self-assigned this Sep 9, 2015

@ghost

This comment has been minimized.

Copy link

ghost commented Sep 9, 2015

@branch-predictor could you add a minimal test case that shows the problem & demonstrate the bug fix ? Otherwise looks great :)

@branch-predictor

This comment has been minimized.

Copy link
Member Author

branch-predictor commented Sep 9, 2015

Actually all it takes is to do

rados -p <pool> bench 10 write --no-cleanup 

on one node, then on another node:

rados -p <pool> bench <any number> seq 

for seq lock failure, and

rados -p <pool> bench <any number> rand 

for the segfault with memcmp. The caveat here is that I managed to hit both cases on one particular host in 4-node cluster, with remaining two other nodes not showing up this bug. On node on which I did rados bench write it obviously worked.

@ghost

This comment has been minimized.

Copy link

ghost commented Sep 9, 2015

The Lock is a race and I can't think of a way to reproduce it reliably.

@ghost

This comment has been minimized.

Copy link

ghost commented Sep 9, 2015

The memcmp crash should be easy to repeat somewhere in src/test/test_rados_tool.sh ?

@branch-predictor

This comment has been minimized.

Copy link
Member Author

branch-predictor commented Sep 9, 2015

Actually, probably yes. Doing bench then truncating all objects down to 0 bytes should do it. I'll check it and update test if it'll work.

@ghost

This comment has been minimized.

Copy link

ghost commented Sep 9, 2015

@branch-predictor it would also work with bench write one object and truncate it. In the spirit of keepinng it minimal.

@branch-predictor

This comment has been minimized.

Copy link
Member Author

branch-predictor commented Sep 9, 2015

@dachary There's already some other test involving rados bench write --no-cleanup, so I could reuse it.

@branch-predictor branch-predictor force-pushed the branch-predictor:bp-rados-fix-noverify branch from b038529 to e6d8097 Sep 9, 2015

@branch-predictor branch-predictor force-pushed the branch-predictor:bp-rados-fix-noverify branch from e6d8097 to 56d0b3a Sep 10, 2015

branch-predictor added some commits Sep 9, 2015

common/obj_bencher.cc: fix verification crashing when there's no objects
When specified pool is empty or doesn't contain valid bench objects, rados
can crash because it attempts to memcmp through NULL pointer. Added check
for valid object size (which also fixes the case when objects are there, but
they're truncated), and made object_size being size_t instead of int.
In seq_read_bench, there's a missing lock.Lock() which causes rados to trigger
assert when object reading fails. Moved lock.Lock() before completion_ret()
call.

Signed-off-by: Piotr Dałek <piotr.dalek@ts.fujitsu.com>
test/test_rados_tool.sh: implement regression test for bench verify c…
…rash

Truncate all objects after format test, then check whether rados bench rand/seq
still works, or just crashes.

Signed-off-by: Piotr Dałek <piotr.dalek@ts.fujitsu.com>

@branch-predictor branch-predictor force-pushed the branch-predictor:bp-rados-fix-noverify branch from 56d0b3a to 612480b Sep 29, 2015

ghost pushed a commit that referenced this pull request Sep 30, 2015

Loic Dachary
Merge pull request #5853 from branch-predictor/bp-rados-fix-noverify
common/obj_bencher.cc: fix verification crashing when there's no objects

Reviewed-by: Loic Dachary <ldachary@redhat.com>

@ghost ghost merged commit b750a16 into ceph:master Sep 30, 2015

@branch-predictor branch-predictor deleted the branch-predictor:bp-rados-fix-noverify branch Feb 1, 2016

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.