At 03/07/2017 03:41 PM, Reshetova, Elena wrote: >> 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. Attached. Which is the generated by running btrfs/070 test case. And I canceled the case almost instantly, so output is not much, but still contains enough info. Both refcount_inc() and refcount_sub_and_test() are causing warning. So now I'm not sure which is the cause, btrfs or bad use of refcount? Thanks, Qu > > 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(-) >>>>> >>>> >>> >>> >>> >> > > >