Project

General

Profile

Bug #7378

ceph --format plain --admin-socket mon.asok crashes the mon

Added by Loic Dachary over 6 years ago. Updated over 6 years ago.

Status:
Resolved
Priority:
Urgent
Assignee:
Category:
Monitor
Target version:
% Done:

100%

Source:
Community (dev)
Tags:
Backport:
emperor, dumpling
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:
Crash signature:

Description

Steps to reproduce, from the src dir on todays master:

rm -fr dev out ;  mkdir -p dev ; LC_ALL=C MON=1 OSD=3 bash -x ./vstart.sh -d -n -X -l mon osd
./ceph --format plain --admin-daemon out/mon.a.asok config get paxos_propose_interval

It should return EINVAL instead of crashing the mon since plain is not supported. It works fine with json
$ ./ceph --format xml --admin-daemon out/mon.a.asok config get paxos_propose_interval
*** DEVELOPER MODE: setting PATH, PYTHONPATH and LD_LIBRARY_PATH ***
{"paxos_propose_interval":"1"}

Associated revisions

Revision 165e76d4 (diff)
Added by Loic Dachary over 6 years ago

common: admin socket fallback to json-pretty format

If the format argument to a command sent to the admin socket is not
among the supported formats ( json, json-pretty, xml, xml-pretty ) the
new_formatter function will return null and the AdminSocketHook::call
function must fall back to a sensible default.

The CephContextHook::call and HelpHook::call failed to do that and a
malformed format argument would cause the mon to crash. A check is added
to each of them and fallback to json-pretty if the format is not
recognized.

To further protect AdminSocketHook::call implementations from similar
problems the format argument is checked immediately after accepting the
command in AdminSocket::do_accept and replaced with json-pretty if it is
not known.

A test case is added for both CephContextHook::call and HelpHook::call
to demonstrate the problem exists and is fixed by the patch.

Three other instances of unsafe calls to new_formatter were found and
a fallback to json-pretty was added. All other calls have been audited
and appear to be safe.

http://tracker.ceph.com/issues/7378 fixes #7378

Backport: emperor, dumpling
Signed-off-by: Loic Dachary <>

Revision 30a604d2 (diff)
Added by Loic Dachary over 6 years ago

common: admin socket fallback to json-pretty format

If the format argument to a command sent to the admin socket is not
among the supported formats ( json, json-pretty, xml, xml-pretty ) the
new_formatter function will return null and the AdminSocketHook::call
function must fall back to a sensible default.

The CephContextHook::call and HelpHook::call failed to do that and a
malformed format argument would cause the mon to crash. A check is added
to each of them and fallback to json-pretty if the format is not
recognized.

To further protect AdminSocketHook::call implementations from similar
problems the format argument is checked immediately after accepting the
command in AdminSocket::do_accept and replaced with json-pretty if it is
not known.

A test case is added for both CephContextHook::call and HelpHook::call
to demonstrate the problem exists and is fixed by the patch.

Three other instances of unsafe calls to new_formatter were found and
a fallback to json-pretty was added. All other calls have been audited
and appear to be safe.

http://tracker.ceph.com/issues/7378 fixes #7378

Signed-off-by: Loic Dachary <>
(cherry picked from commit 165e76d4d03ffcc490fd3c2ba60fb37372990d0a)

Revision 04e78f89 (diff)
Added by Loic Dachary over 6 years ago

common: admin socket fallback to json-pretty format

If the format argument to a command sent to the admin socket is not
among the supported formats ( json, json-pretty, xml, xml-pretty ) the
new_formatter function will return null and the AdminSocketHook::call
function must fall back to a sensible default.

The CephContextHook::call and HelpHook::call failed to do that and a
malformed format argument would cause the mon to crash. A check is added
to each of them and fallback to json-pretty if the format is not
recognized.

To further protect AdminSocketHook::call implementations from similar
problems the format argument is checked immediately after accepting the
command in AdminSocket::do_accept and replaced with json-pretty if it is
not known.

A test case is added for both CephContextHook::call and HelpHook::call
to demonstrate the problem exists and is fixed by the patch.

Three other instances of unsafe calls to new_formatter were found and
a fallback to json-pretty was added. All other calls have been audited
and appear to be safe.

http://tracker.ceph.com/issues/7378 fixes #7378

Signed-off-by: Loic Dachary <>
(cherry picked from commit 165e76d4d03ffcc490fd3c2ba60fb37372990d0a)

History

#1 Updated by Loic Dachary over 6 years ago

  • Description updated (diff)

#2 Updated by Ian Colle over 6 years ago

  • Assignee set to Joao Eduardo Luis
  • Priority changed from Normal to Urgent

#3 Updated by Loic Dachary over 6 years ago

For what it's worth, it's not a blocker for me. The "plain" format is documented to not being supported and it only happens if you have access to the admin socket. In other words, I was looking for troubles and stumbled on what appears to be a bug.

#4 Updated by Loic Dachary over 6 years ago

  • Assignee changed from Joao Eduardo Luis to Loic Dachary

#5 Updated by Loic Dachary over 6 years ago

  • Status changed from New to Fix Under Review

#6 Updated by Loic Dachary over 6 years ago

  • Target version set to v0.77
  • % Done changed from 0 to 90
  • Source changed from other to Community (dev)
  • Backport set to emperor, dumpling

#7 Updated by Loic Dachary over 6 years ago

Line numbers are relative to b9a127e

src/client/Client.cc:113 is not safe
src/osdc/Objecter.cc:2509 is not safe
src/osd/OSD.cc:912 is not safe because dump_ops_in_flight does not test if f is null
src/mon/AuthMonitor.cc contains > 10 if to deal with the fact that f is null or not
src/mon/MDSMonitor.cc contains < 10 if to deal with the fact that f is null or not
src/mon/Monitor.cc contains > 10 if to deal with the fact that f is null or not
src/mon/MonmapMonitor.cc contains < 10 if to deal with the fact that f is null or not
src/mon/OSDMonitor.cc:2195 and src/mon/OSDMonitor.cc:2218 have an unsafe call where there is no risk to reach it http://tracker.ceph.com/issues/7388
src/mon/Monitor.cc contains > 10 if to deal with the fact that f is null or not
src/mon/PGMonitor.cc contains < 10 if to deal with the fact that f is null or not
src/osd/OSD.cc contains < 10 if to deal with the fact that f is null or not
src/osd/ReplicatedPG.cc contains < 10 if to deal with the fact that f is null or not
src/test/bench/small_io_bench_fs.cc src/test/common/get_command_descriptions.cc src/test/common/test_sloppy_crc_map.cc and src/test/crush/indep.cc use hard-coded arguments

There is a total ol 44 call to new_formatter found in master but it appears that there are various strategies to deal with the return of a null formatter, a number of which are custom. For instance osd dump will print() if the formatter is null while osd crush rule list will fall back to json_pretty.

Adding an argument to new_formatter would require to go over all calls to new_formatter, add NULL where it appears to handle the case where it returns null and add the default where it does not. Another solution is to modify the three calls that are unsafe.

#8 Updated by Loic Dachary over 6 years ago

  • Status changed from Fix Under Review to Resolved
  • % Done changed from 90 to 100

Also available in: Atom PDF