[PATCH 0/2] ceph: don't let a bad mon_addr= reach the empty-monmap BUG_ON()

Yogesh Gaur posted 2 patches 1 week, 6 days ago
fs/ceph/super.c       | 9 +++++++--
net/ceph/mon_client.c | 3 +++
2 files changed, 10 insertions(+), 2 deletions(-)
[PATCH 0/2] ceph: don't let a bad mon_addr= reach the empty-monmap BUG_ON()
Posted by Yogesh Gaur 1 week, 6 days ago
syzbot can panic the kernel from fsconfig(2) by mounting cephfs with a
mon_addr= that does not parse:

  kernel BUG at net/ceph/mon_client.c:211!
  RIP: 0010:pick_new_mon net/ceph/mon_client.c:211 [inline]
  RIP: 0010:__open_session+0x60f/0x740 net/ceph/mon_client.c:245
   ceph_monc_open_session+0x46/0x60 net/ceph/mon_client.c:534
   __ceph_open_session+0x135/0x590 net/ceph/ceph_common.c:798
   ceph_real_mount fs/ceph/super.c:1173 [inline]
   ceph_get_tree+0xd98/0x1fd0 fs/ceph/super.c:1362
   vfs_cmd_create+0xd7/0x2a0 fs/fsopen.c:231
   __do_sys_fsconfig+0x55a/0xcb0 fs/fsopen.c:463

  https://syzkaller.appspot.com/bug?extid=1dbed5969931c19d3eb5

The sequence is fsopen("ceph"), fsconfig(FSCONFIG_SET_STRING, "mon_addr",
<something that does not resolve>) - which returns an error that the
caller simply ignores - and then FSCONFIG_CMD_CREATE. Two things have to
be true for that to end in a BUG():

 1/ ceph_parse_mon_addr() commits the string to fsopt->mon_addr before
    validating it, so the rejected value survives the failed fsconfig()
    and satisfies ceph_get_tree()'s "No monitor address" check.

 2/ ceph_parse_ips() never got as far as writing *count, so num_mon stays
    0, and build_initial_monmap() happily builds a monmap with no
    monitors in it. pick_new_mon() then trips its own invariant.

Patch 1 closes 2/ in libceph, which is what actually stops the BUG().
It is the same rule commit 40480eee361e ("libceph: Reject monmaps
advertising zero monitors") applied to monmaps arriving from the wire,
now applied to the one built locally from mount options. Patch 2 closes
1/ so the bad string is not kept in the first place and the failure is
reported where it happens.

Either patch on its own stops the reported crash; I think both are worth
having, but say the word and I will send just the first.

Compile-tested only: x86_64 defconfig + CONFIG_CEPH_LIB=m +
CONFIG_CEPH_FS=m, net/ceph/mon_client.o and fs/ceph/super.o both W=1
clean, checkpatch --strict clean on both patches. I have no ceph cluster
here and did not run the syzbot reproducer, so this is reasoning from the
code and the report, not an observed fix - please treat it accordingly.

Yogesh Gaur (2):
  libceph: reject an initial monmap with no monitors
  ceph: don't keep a mon_addr= that failed to parse

 fs/ceph/super.c       | 9 +++++++--
 net/ceph/mon_client.c | 3 +++
 2 files changed, 10 insertions(+), 2 deletions(-)

-- 
2.55.0.windows.5