cephfs: fix write_buf's _len overflow problem#13587
cephfs: fix write_buf's _len overflow problem#13587jcsp merged 1 commit intoceph:masterfrom yanghonggang:master
Conversation
|
@jcsp Your comments and suggestions will be greatly appreciated. |
|
please split the change into three commit. one for buffer.{h,cc}, one for Journaler.{h,cc} and one for Server.cc |
|
@ukernel done! |
|
@ukernel does this look good to you? |
|
@liewegas @ukernel @jcsp @gregsfortytwo Normal user of our fs can damage the whole system by writing huge xattr kv pairs. I think this is a serious bug. Your quick reply can help me to fix this bug as soon as possible. Thank you for your time to review this patch. |
src/common/buffer.cc
Outdated
| char* ptr = _raw->data + _off + _len; | ||
| *ptr = c; | ||
| _len++; | ||
| assert(_len != 0); |
There was a problem hiding this comment.
I'd prefer if these asserts are compiled conditionally, for full debug builds only. This is already performance sensitive path that affects everyone.
There was a problem hiding this comment.
Yes, this will affect performance. I think assert can be disabled globally by specifing DCMAKE_CXX_FLAGS="-DNDEBUG ..." cmake variable. Any better suggestion?
|
The Server and Journaler commits look good. After chatting with @liewegas the consensus on the buffer.h asserts is that although we already have lots of asserts there, we probably don't want to add more. If you could drop the buffer.h commit from this PR, it will be good to merge. |
|
@jcsp @branch-predictor @ukernel |
src/mds/Server.cc
Outdated
|
|
||
| // check xattrs kv pairs size | ||
| size_t cur_xattrs_size = 0; | ||
| for (map<string, bufferptr>::iterator p = pxattrs->begin(); |
There was a problem hiding this comment.
i'd recommend use the range-based loop when possible.
src/mds/Server.cc
Outdated
| int max_bytes = req->head.args.readdir.max_bytes; | ||
| if (!max_bytes) | ||
| max_bytes = 512 << 10; | ||
| max_bytes = (512 << 10) + g_conf->mds_max_xattr_pairs_size; // make sure at least one item can be encoded |
After I have set about 400 64KB xattr kv pair to a file, mds is crashed. Every time I try to start mds, it will crash again. The root reason is write_buf._len overflowed when doing Journaler::append_entry(). This patch try to fix this problem through the following changes: 1. limit file/dir's xattr size 2. throttle journal entry append operations Fixes: http://tracker.ceph.com/issues/19033 Signed-off-by: Yang Honggang joseph.yang@xtaotech.com
|
@tchaikov |
After I have set about 400 64KB xattr kv pair to a file,
mds is crashed. Every time I try to start mds, it will crash again.
The root reason is write_buf._len overflowed when doing
Journaler::append_entry().
This patch try to fix this problem through the following changes:
Fixes: http://tracker.ceph.com/issues/19033
Signed-off-by: Yang Honggang joseph.yang@xtaotech.com