On Saturday, September 21st, 2024 at 8:47 PM, Kent Overstreet <[email protected]> wrote:
> On Mon, Sep 16, 2024 at 10:47:57PM GMT, Piotr Zalewski wrote: > > > Add __GFP_ZERO flag to kvmalloc call in btree_bounce_alloc to mitigate > > later uinit-value use KMSAN warning[1]. > > > > After applying the patch reproducer still triggers stack overflow[2] but > > it seems unrelated to the uninit-value use warning. After further > > investigation it was found that stack overflow occurs because KMSAN adds > > additional function calls. Backtrace of where the stack magic number gets > > smashed was added as a reply to syzkaller thread[3]. > > > > I confirmed that task's stack magic number gets smashed after the code path > > where KSMAN detects uninit-value use is executed, so it can be assumed that > > it doesn't contribute in any way to uninit-value use detection. > > > > [1] https://syzkaller.appspot.com/bug?extid=6f655a60d3244d0c6718 > > [2] > > https://lore.kernel.org/lkml/[email protected] > > [3] > > https://lore.kernel.org/all/rVaWgPULej8K7HqMPNIu8kVNyXNjjCiTB-QBtItLFBmk0alH6fV2tk4joVPk97Evnuv4ZRDd8HB5uDCkiFG6u81xKdzDj-KrtIMJSlF6Kt8=@proton.me > > > > Signed-off-by: Piotr Zalewski [email protected] > > > Oh hey, nice find :) Hi! > We should be able to fix this in a more performant way, though; btree > node resort is a path where we do care about performance, we don't want > to touch the whole buffer more times than necessary. > > Can you try zeroing out the portion after what we consumed, after we > sort into the bounce buffer? Do you mean something like this? : diff --git a/fs/bcachefs/btree_io.c b/fs/bcachefs/btree_io.c index 56ea9a77cd4a..c737ece6f628 100644 --- a/fs/bcachefs/btree_io.c +++ b/fs/bcachefs/btree_io.c @@ -1195,6 +1195,10 @@ int bch2_btree_node_read_done(struct bch_fs *c, struct bch_dev *ca, set_btree_bset(b, b->set, &b->data->keys); b->nr = bch2_key_sort_fix_overlapping(c, &sorted->keys, iter); + memset((uint8_t*)(sorted + 1) + b->nr.live_u64s * sizeof(u64), 0, + btree_buf_bytes(b) - + sizeof(struct btree_node) - + b->nr.live_u64s * sizeof(u64)); u64s = le16_to_cpu(sorted->keys.u64s); *sorted = *b->data; I tested that above doesn't trigger uinit-value usage. Best regards, Piotr Zalewski
