> At 03/06/2017 05:43 PM, Reshetova, Elena wrote: > > > >> At 03/03/2017 04:55 PM, Elena Reshetova wrote: > >>> Now when new refcount_t type and API are finally merged > >>> (see include/linux/refcount.h), the following > >>> patches convert various refcounters in the btrfs filesystem from atomic_t > >>> to refcount_t. By doing this we prevent intentional or accidental > >>> underflows or overflows that can led to use-after-free vulnerabilities. > >>> > >>> The below patches are fully independent and can be cherry-picked separately. > >>> Since we convert all kernel subsystems in the same fashion, resulting > >>> in about 300 patches, we have to group them for sending at least in some > >>> fashion to be manageable. Please excuse the long cc list. > >>> > >>> These patches have been tested with xfstests by running btrfs-related tests. > >>> btrfs debug was enabled, warns on refcount errors, too. No output related to > >>> refcount errors produced. However, the following errors were during the run: > >>> * tests btrfs/078, btrfs/114, btrfs/115, no errors anywhere in dmesg, but > >>> process hangs. They all seem to be around qgroup, sometimes error visible > >>> such as qgroup scan failed -4 before it blocks, but not always. > >> > >> How reproducible of the hang? > > > > Always in my environment, but I would not much go into investigating why it > happens, if it works for you. > > My test environment is far from ideal: I am testing in VM with rather old > userspace and couple of additional changes in, > > so there are many things that can potentially go wrong. Anyway the strace for > 078 is in the attachment. > > Thanks for the strace. > > However no "-f" is passed to strace, so it doesn't contain much useful info. > > > > > If the patches pass all tests on your side, could you please take them in and > propagate further? > > I will continue with other kernel subsystems. > > The patchset itself looks like a common cleanup, while I did encounter > several cases (almost all scrub tests) causing kernel warning due to > underflow. Oh, could you please send me the warning outputs? I can hopefully analyze and fix them. Best Regards, Elena. > > So I'm afraid the patchset will not be merged until we fix all the > underflows. > > But thanks for the patchset, it helps us to expose a lot of problem. > > Thanks, > Qu > > > > > Best Regards, > > Elena. > > > > > >> > >> I also see the -EINTR output, but that seems to be designed for > >> btrfs/11[45]. > >> > >> btrfs/078 is unrelated to qgroup, and all these three test pass in my > >> test environment, which is v4.11-rc1 with your patches applied. > >> > >> I ran these 3 tests in a row with default and space_cache=v2 mount > >> options, and 5 times for each mount option, no hang at all. > >> > >> It would help much if more info can be provided, from blocked process > >> backtrace to test mount option to base commit. > >> > >> Thanks, > >> Qu > >> > >>> * test btrfs/104 dmesg has additional error output: > >>> BTRFS warning (device vdc): qgroup 258 reserved space underflow, have: 0, > >>> to free: 4096 > >>> I tried looking at the code on what causes the failure, but could not figure > >>> it out. It doesn't seem to be related to any refcount changes at least IMO. > >>> > >>> The above test failures are hard for me to understand and interpreted, but > >>> they don't seem to relate to refcount conversions. > >>> > >>> Elena Reshetova (17): > >>> fs, btrfs: convert btrfs_bio.refs from atomic_t to refcount_t > >>> fs, btrfs: convert btrfs_transaction.use_count from atomic_t to > >>> refcount_t > >>> fs, btrfs: convert extent_map.refs from atomic_t to refcount_t > >>> fs, btrfs: convert btrfs_ordered_extent.refs from atomic_t to > >>> refcount_t > >>> fs, btrfs: convert btrfs_caching_control.count from atomic_t to > >>> refcount_t > >>> fs, btrfs: convert btrfs_delayed_ref_node.refs from atomic_t to > >>> refcount_t > >>> fs, btrfs: convert btrfs_delayed_node.refs from atomic_t to refcount_t > >>> fs, btrfs: convert btrfs_delayed_item.refs from atomic_t to refcount_t > >>> fs, btrfs: convert btrfs_root.refs from atomic_t to refcount_t > >>> fs, btrfs: convert extent_state.refs from atomic_t to refcount_t > >>> fs, btrfs: convert compressed_bio.pending_bios from atomic_t to > >>> refcount_t > >>> fs, btrfs: convert scrub_recover.refs from atomic_t to refcount_t > >>> fs, btrfs: convert scrub_page.refs from atomic_t to refcount_t > >>> fs, btrfs: convert scrub_block.refs from atomic_t to refcount_t > >>> fs, btrfs: convert scrub_parity.refs from atomic_t to refcount_t > >>> fs, btrfs: convert scrub_ctx.refs from atomic_t to refcount_t > >>> fs, btrfs: convert btrfs_raid_bio.refs from atomic_t to refcount_t > >>> > >>> fs/btrfs/backref.c | 2 +- > >>> fs/btrfs/compression.c | 18 ++++++++--------- > >>> fs/btrfs/ctree.h | 5 +++-- > >>> fs/btrfs/delayed-inode.c | 46 ++++++++++++++++++++++---------------------- > >>> fs/btrfs/delayed-inode.h | 5 +++-- > >>> fs/btrfs/delayed-ref.c | 8 ++++---- > >>> fs/btrfs/delayed-ref.h | 8 +++++--- > >>> fs/btrfs/disk-io.c | 6 +++--- > >>> fs/btrfs/disk-io.h | 4 ++-- > >>> fs/btrfs/extent-tree.c | 20 +++++++++---------- > >>> fs/btrfs/extent_io.c | 18 ++++++++--------- > >>> fs/btrfs/extent_io.h | 3 ++- > >>> fs/btrfs/extent_map.c | 10 +++++----- > >>> fs/btrfs/extent_map.h | 3 ++- > >>> fs/btrfs/ordered-data.c | 20 +++++++++---------- > >>> fs/btrfs/ordered-data.h | 2 +- > >>> fs/btrfs/raid56.c | 19 +++++++++--------- > >>> fs/btrfs/scrub.c | 42 ++++++++++++++++++++-------------------- > >>> fs/btrfs/transaction.c | 20 +++++++++---------- > >>> fs/btrfs/transaction.h | 3 ++- > >>> fs/btrfs/tree-log.c | 2 +- > >>> fs/btrfs/volumes.c | 10 +++++----- > >>> fs/btrfs/volumes.h | 2 +- > >>> include/trace/events/btrfs.h | 4 ++-- > >>> 24 files changed, 143 insertions(+), 137 deletions(-) > >>> > >> > > > > > > >
