btrfs-progs: docs: add extra LLM disclosure following kernel guidelines - #1154
btrfs-progs: docs: add extra LLM disclosure following kernel guidelines#1154adam900710 wants to merge 5 commits into
Conversation
In kernel-shared/print-tree.c, we have a lot of forced type casting to "(unsigned long long)" for the format "%llu", no matter if the original value is really u64. Change all those forced type casting used in printing functions, to use the correct native type instead. Assisted-by: LLM (coding) Signed-off-by: Qu Wenruo <wqu@suse.com>
[BUG]
Running "btrfs rescue fix-data-checksum" on a filesystem with a data
checksum mismatch triggers a heap buffer overflow:
==3692==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x502000000190
READ of size 8 at 0x502000000190 thread T0
#0 set_bit kernel-lib/bitops.h:32
#1 add_corrupted_block cmds/rescue-fix-data-checksum.c:99
kdave#2 verify_one_data_block cmds/rescue-fix-data-checksum.c:143
kdave#3 iterate_one_csum_item cmds/rescue-fix-data-checksum.c:178
kdave#4 iterate_csum_root cmds/rescue-fix-data-checksum.c:252
kdave#5 btrfs_recover_fix_data_checksum cmds/rescue-fix-data-checksum.c:510
0x502000000191 is located 0 bytes after 1-byte region [0x502000000190,0x502000000191)
allocated by thread T0 here:
#0 calloc
#1 add_corrupted_block cmds/rescue-fix-data-checksum.c:94
[CAUSE]
BITS_TO_LONGS() returns the number of unsigned longs needed to hold the
given amount of bits, not the number of bytes. But the value is passed as
the byte size to calloc():
last->error_mirror_bitmap = calloc(1, BITS_TO_LONGS(num_mirrors));
For any sane number of mirrors this allocates a single byte, while
set_bit() accesses the bitmap in unsigned long units, thus reading and
writing 8 bytes out of a 1 byte allocation.
[FIX]
Use bitmap_zalloc() from kernel-lib/bitmap.h, which does the size
calculation for the caller, and pair it with bitmap_free() when the
record is released.
Fixes: 0e999c9 ("btrfs-progs: introduce "btrfs rescue fix-data-checksum"")
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
… bitmap
[BUG]
With both mirrors of a DUP data block corrupted, only the first one shows
up in the report:
# btrfs rescue fix-data-checksum dup.img
logical=84082688 corrtuped mirrors=1 affected files:
(subvolume 5)/file2
While both mirrors have a checksum mismatch, so the expected output is:
logical=84082688 corrtuped mirrors=1,2 affected files:
(subvolume 5)/file2
[CAUSE]
The error mirror bitmap uses bit 0 for mirror 1, as documented in struct
corrupted_block, and add_corrupted_block() does that conversion when it
allocates a new entry:
set_bit(mirror - 1, last->error_mirror_bitmap);
But when an entry for the same logical bytenr already exists, the mirror
number is used as the bit number directly:
set_bit(mirror, last->error_mirror_bitmap);
The first corrupted mirror of a block always goes through the allocation
path, so only the second and any following one are affected, and their bit
ends up one position too high.
For the last mirror this means bit @num_mirrors gets set, which is beyond
the range report_corrupted_blocks() walks, so the mirror is dropped from
the report. For the other mirrors the number reported is one larger than
the corrupted one, e.g. on a RAID1C3 block with mirrors 1 and 2 corrupted
the report claims mirrors 1 and 3, marking the only intact mirror as bad.
This is more than a cosmetic problem, as the report is what the user bases
the mirror choice on in the interactive mode. Picking a mirror that is
reported as intact but is in fact corrupted updates the checksum item to
match the corrupted data, making the corruption permanent.
[FIX]
Convert the mirror number to a bit number the same way the allocation path
does.
Fixes: 0e999c9 ("btrfs-progs: introduce "btrfs rescue fix-data-checksum"")
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
The core design of btrfs qgroup mode is to track the ownership of every extent accurately, but that is conflicting with snapshots, which can change the ownership of tons of extents in a very short time, during snapshot creation and deletion. Although qgroup mode has several workarounds, it is not fully resolved and still introduces extra costs, e.g. requires frequent rescan, breaking limits etc. So make it more clear that qgroup mode is not recommended until the end user has a rigid subvolume layout, e.g. no more new subvolume/snapshots, nor deletion of any existing subvolume. Signed-off-by: Qu Wenruo <wqu@suse.com>
There are increased LLM assisted bug fixes in the mailing list, and I have a very strong feeling that most of the new bug fixes from unknown developers are at least assisted of LLM, and they never properly disclose the usage of LLM. I don't know much difference such updated docs will make, but at least let's try to make them disclose the LLM usage. Assisted-by: LLM (grammar fixes) Signed-off-by: Qu Wenruo <wqu@suse.com>
e364c16 to
705d903
Compare
|
At this moment I don't care much if AI/LLM is used for patches, they get the same level of review, new/unknown people or not. This is a very divisive topic, I don't have as strict requirements as kernel but at least for licensing and human behind the actual pull request or submission it makes sense. Mentioning which tool/model was used has little value other than giving credit, or advertising. Unlike other targeted tools like coccinelle or sparse, the AI models can do anything, unless the patch generation is reproducible it does not help us much. |
|
This doc is not only for btrfs-progs, but also kernel contributions. The reasons I want to proper LLM disclosure are:
Sure, we keep the same level of review no matter if it's generated by human or LLM, but the trend is already there, those new/unknown authors are completely relying on LLM to expose bugs with little ability to defend whatever they submit. That's why I want to proper LLM disclosure other than not. I do not care about what model they are using, I only care about if they are some LLM-era script kiddies. |
3 participants
There are increased LLM assisted bug fixes in the mailing list, and I have a very strong feeling that most of the new bug fixes from unknown developers are at least assisted of LLM, and they never properly disclose the usage of LLM.
I don't know much difference such updated docs will make, but at least let's try to make them disclose the LLM usage.