April 2022 - Linux-stable-mirror

[PATCH] drivers: nfc: nfcmrvl: fix bugs caused by fw_dnld_over in nfcmrvl driver

by Duoming Zhou

The fw_dnld_over() is a reentrant function in nfcmrvl driver, it could be called by nfcmrvl_fw_dnld_start(), nfcmrvl_nci_recv_frame() and nfcmrvl_nci_unregister_dev() without synchronization. As a result, the firmware struct could be deallocated more than once, which leads to double-free or invalid-free bugs. The first situation that causes bug is shown below: (Thread 1) | (Thread 2) nfcmrvl_fw_dnld_start | ... | nfcmrvl_nci_unregister_dev release_firmware() | nfcmrvl_fw_dnld_abort kfree(fw) //(1) | fw_dnld_over | release_firmware ... | kfree(fw) //(2) | ... The second situation that causes bug is shown below: (Thread 1) | (Thread 2) nfcmrvl_fw_dnld_start | ... | mod_timer | (wait a time) | fw_dnld_timeout | nfcmrvl_nci_unregister_dev fw_dnld_over | nfcmrvl_fw_dnld_abort release_firmware | fw_dnld_over kfree(fw) //(1) | release_firmware ... | kfree(fw) //(2) The third situation that causes bug is shown below: (Thread 1) | (Thread 2) nfcmrvl_nci_recv_frame | if(..->fw_download_in_progress)| nfcmrvl_fw_dnld_recv_frame | queue_work | | fw_dnld_rx_work | nfcmrvl_nci_unregister_dev fw_dnld_over | nfcmrvl_fw_dnld_abort release_firmware | fw_dnld_over kfree(fw) //(1) | release_firmware | kfree(fw) //(2) The firmware struct is deallocated in position (1) and deallocated in position (2) again. The crash trace triggered by POC is like below: [ 122.640457] BUG: KASAN: double-free or invalid-free in fw_dnld_over+0x28/0xf0 [ 122.640457] Call Trace: [ 122.640457] <TASK> [ 122.640457] kfree+0xb0/0x330 [ 122.640457] fw_dnld_over+0x28/0xf0 [ 122.640457] nfcmrvl_nci_unregister_dev+0x61/0x70 [ 122.640457] nci_uart_tty_close+0x87/0xd0 [ 122.640457] tty_ldisc_kill+0x3e/0x80 [ 122.640457] tty_ldisc_hangup+0x1b2/0x2c0 [ 122.640457] __tty_hangup.part.0+0x316/0x520 [ 122.640457] tty_release+0x200/0x670 [ 122.640457] __fput+0x110/0x410 [ 122.640457] task_work_run+0x86/0xd0 [ 122.640457] exit_to_user_mode_prepare+0x1aa/0x1b0 [ 122.640457] syscall_exit_to_user_mode+0x19/0x50 [ 122.640457] do_syscall_64+0x48/0x90 [ 122.640457] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 122.640457] RIP: 0033:0x7f68433f6beb What's more, there are also use-after-free and null-ptr-deref bugs in nfcmrvl_fw_dnld_start(). If we deallocate firmware struct or set null to the members of priv->fw_dnld in fw_dnld_over(), then, we dereference firmware or the members of priv->fw_dnld in nfcmrvl_fw_dnld_start(), the UAF or NPD bugs will happen. This patch reorders nfcmrvl_fw_dnld_abort() after nci_unregister_device() to avoid the double-free, UAF and NPD bugs, as nci_unregister_device() is well synchronized and won't return if there is a running routine. Fixes: 3194c6870158 ("NFC: nfcmrvl: add firmware download support") Signed-off-by: Duoming Zhou <duoming(a)zju.edu.cn> Cc: stable(a)vger.kernel.org --- drivers/nfc/nfcmrvl/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nfc/nfcmrvl/main.c b/drivers/nfc/nfcmrvl/main.c index 2fcf545012b..1a5284de434 100644 --- a/drivers/nfc/nfcmrvl/main.c +++ b/drivers/nfc/nfcmrvl/main.c @@ -183,6 +183,7 @@ void nfcmrvl_nci_unregister_dev(struct nfcmrvl_private *priv) { struct nci_dev *ndev = priv->ndev; + nci_unregister_device(ndev); if (priv->ndev->nfc_dev->fw_download_in_progress) nfcmrvl_fw_dnld_abort(priv); @@ -191,7 +192,6 @@ void nfcmrvl_nci_unregister_dev(struct nfcmrvl_private *priv) if (gpio_is_valid(priv->config.reset_n_io)) gpio_free(priv->config.reset_n_io); - nci_unregister_device(ndev); nci_free_device(ndev); kfree(priv); } -- 2.17.1

3 years

1
0
0 0

[PATCH] drivers: nfc: nfcmrvl: fix double-free bugs in nfcmrvl driver

by Duoming Zhou

There are double-free bugs in nfcmrvl driver. The root cause of these bugs is that the functions call release_firmware() in nfcmrvl driver is not well synchronized. The double-free bugs are between nfcmrvl_fw_dnld_start() and nfcmrvl_nci_unregister_dev(). Both of these functions will call release_firmware(). As a result, the firmware struct will be deallocated more than once. One double-free bug is shown below: (Thread 1) | (Thread 2) nfcmrvl_fw_dnld_start | ... | nfcmrvl_nci_unregister_dev release_firmware() | nfcmrvl_fw_dnld_abort kfree(fw) //(1) | fw_dnld_over | release_firmware | kfree(fw) //(2) ... | ... Another double-free bug is shown below: (Thread 1) | (Thread 2) nfcmrvl_fw_dnld_start | ... | mod_timer | (wait a time) | fw_dnld_timeout | nfcmrvl_nci_unregister_dev fw_dnld_over | nfcmrvl_fw_dnld_abort release_firmware | fw_dnld_over kfree(fw) //(1) | release_firmware ... | kfree(fw) //(2) The firmware struct is deallocated in position (1) and deallocated in position (2) again. The crash trace triggered by POC is like below: [ 122.640457] BUG: KASAN: double-free or invalid-free in fw_dnld_over+0x28/0xf0 [ 122.640457] Call Trace: [ 122.640457] <TASK> [ 122.640457] kfree+0xb0/0x330 [ 122.640457] fw_dnld_over+0x28/0xf0 [ 122.640457] nfcmrvl_nci_unregister_dev+0x61/0x70 [ 122.640457] nci_uart_tty_close+0x87/0xd0 [ 122.640457] tty_ldisc_kill+0x3e/0x80 [ 122.640457] tty_ldisc_hangup+0x1b2/0x2c0 [ 122.640457] __tty_hangup.part.0+0x316/0x520 [ 122.640457] tty_release+0x200/0x670 [ 122.640457] __fput+0x110/0x410 [ 122.640457] task_work_run+0x86/0xd0 [ 122.640457] exit_to_user_mode_prepare+0x1aa/0x1b0 [ 122.640457] syscall_exit_to_user_mode+0x19/0x50 [ 122.640457] do_syscall_64+0x48/0x90 [ 122.640457] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 122.640457] RIP: 0033:0x7f68433f6beb What's more, there are also use-after-free and null-ptr-deref bugs in nfcmrvl_fw_dnld_start(). If we deallocate firmware struct or set null to the members of priv->fw_dnld in nfcmrvl_nci_unregister_dev(), then, we dereference firmware or the members of priv->fw_dnld in nfcmrvl_fw_dnld_start(), the UAF or NPD bugs will happen. This patch reorders nfcmrvl_fw_dnld_abort() after nci_unregister_device() to avoid the double-free, UAF and NPD bugs, as nci_unregister_device() is well synchronized and won't return if there is a running routine. Fixes: 3194c6870158 ("NFC: nfcmrvl: add firmware download support") Signed-off-by: Duoming Zhou <duoming(a)zju.edu.cn> Cc: stable(a)vger.kernel.org --- drivers/nfc/nfcmrvl/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nfc/nfcmrvl/main.c b/drivers/nfc/nfcmrvl/main.c index 2fcf545012b..1a5284de434 100644 --- a/drivers/nfc/nfcmrvl/main.c +++ b/drivers/nfc/nfcmrvl/main.c @@ -183,6 +183,7 @@ void nfcmrvl_nci_unregister_dev(struct nfcmrvl_private *priv) { struct nci_dev *ndev = priv->ndev; + nci_unregister_device(ndev); if (priv->ndev->nfc_dev->fw_download_in_progress) nfcmrvl_fw_dnld_abort(priv); @@ -191,7 +192,6 @@ void nfcmrvl_nci_unregister_dev(struct nfcmrvl_private *priv) if (gpio_is_valid(priv->config.reset_n_io)) gpio_free(priv->config.reset_n_io); - nci_unregister_device(ndev); nci_free_device(ndev); kfree(priv); } -- 2.17.1

3 years

1
0
0 0

[PATCH AUTOSEL 5.10 01/18] ARM: vexpress/spc: Avoid negative array index when !SMP

by Sasha Levin

From: Kees Cook <keescook(a)chromium.org> [ Upstream commit b3f1dd52c991d79118f35e6d1bf4d7cb09882e38 ] When building multi_v7_defconfig+CONFIG_SMP=n, -Warray-bounds exposes a couple negative array index accesses: arch/arm/mach-vexpress/spc.c: In function 've_spc_clk_init': arch/arm/mach-vexpress/spc.c:583:21: warning: array subscript -1 is below array bounds of 'bool[2]' {aka '_Bool[2]'} [-Warray-bounds] 583 | if (init_opp_table[cluster]) | ~~~~~~~~~~~~~~^~~~~~~~~ arch/arm/mach-vexpress/spc.c:556:7: note: while referencing 'init_opp_table' 556 | bool init_opp_table[MAX_CLUSTERS] = { false }; | ^~~~~~~~~~~~~~ arch/arm/mach-vexpress/spc.c:592:18: warning: array subscript -1 is below array bounds of 'bool[2]' {aka '_Bool[2]'} [-Warray-bounds] 592 | init_opp_table[cluster] = true; | ~~~~~~~~~~~~~~^~~~~~~~~ arch/arm/mach-vexpress/spc.c:556:7: note: while referencing 'init_opp_table' 556 | bool init_opp_table[MAX_CLUSTERS] = { false }; | ^~~~~~~~~~~~~~ Skip this logic when built !SMP. Link: https://lore.kernel.org/r/20220331190443.851661-1-keescook@chromium.org Cc: Liviu Dudau <liviu.dudau(a)arm.com> Cc: Sudeep Holla <sudeep.holla(a)arm.com> Cc: Lorenzo Pieralisi <lorenzo.pieralisi(a)arm.com> Cc: Russell King <linux(a)armlinux.org.uk> Cc: linux-arm-kernel(a)lists.infradead.org Acked-by: Liviu Dudau <liviu.dudau(a)arm.com> Signed-off-by: Kees Cook <keescook(a)chromium.org> Signed-off-by: Sudeep Holla <sudeep.holla(a)arm.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- arch/arm/mach-vexpress/spc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mach-vexpress/spc.c b/arch/arm/mach-vexpress/spc.c index 1da11bdb1dfb..1c6500c4e6a1 100644 --- a/arch/arm/mach-vexpress/spc.c +++ b/arch/arm/mach-vexpress/spc.c @@ -580,7 +580,7 @@ static int __init ve_spc_clk_init(void) } cluster = topology_physical_package_id(cpu_dev->id); - if (init_opp_table[cluster]) + if (cluster < 0 || init_opp_table[cluster]) continue; if (ve_init_opp_table(cpu_dev)) -- 2.35.1

3 years

2
19
0 0

[PATCH v2 1/3] btrfs: avoid double clean up when submit_one_bio() failed

by Qu Wenruo

[BUG] When running generic/475 with 64K page size and 4K sector size, it has a very high chance (almost 100%) to hang, with mostly data page locked but no one is going to unlock it. [CAUSE] With commit 1784b7d502a9 ("btrfs: handle csum lookup errors properly on reads"), if we failed to lookup checksum due to metadata IO error, we will return error for btrfs_submit_data_bio(). This will cause the page to be unlocked twice in btrfs_do_readpage(): btrfs_do_readpage() |- submit_extent_page() | |- submit_one_bio() | |- btrfs_submit_data_bio() | |- if (ret) { | |- bio->bi_status = ret; | |- bio_endio(bio); } | In the endio function, we will call end_page_read() | and unlock_extent() to cleanup the subpage range. | |- if (ret) { |- unlock_extent(); end_page_read() } Here we unlock the extent and cleanup the subpage range again. For unlock_extent(), it's mostly double unlock safe. But for end_page_read(), it's not, especially for subpage case, as for subpage case we will call btrfs_subpage_end_reader() to reduce the reader number, and use that to number to determine if we need to unlock the full page. If double accounted, it can underflow the number and leave the page locked without anyone to unlock it. [FIX] The commit 1784b7d502a9 ("btrfs: handle csum lookup errors properly on reads") itself is completely fine, it's our existing code not properly handling the error from bio submission hook properly. This patch will make submit_one_bio() to return void so that the callers will never be able to do cleanup when bio submission hook fails. CC: stable(a)vger.kernel.org # 5.18+ Signed-off-by: Qu Wenruo <wqu(a)suse.com> --- fs/btrfs/extent_io.c | 114 ++++++++++++++----------------------------- fs/btrfs/extent_io.h | 3 +- fs/btrfs/inode.c | 13 +++-- 3 files changed, 44 insertions(+), 86 deletions(-) diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 15a429e28174..163aa6dee987 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -166,24 +166,27 @@ static int add_extent_changeset(struct extent_state *state, u32 bits, return ret; } -int __must_check submit_one_bio(struct bio *bio, int mirror_num, - unsigned long bio_flags) +void submit_one_bio(struct bio *bio, int mirror_num, unsigned long bio_flags) { - blk_status_t ret = 0; struct extent_io_tree *tree = bio->bi_private; bio->bi_private = NULL; /* Caller should ensure the bio has at least some range added */ ASSERT(bio->bi_iter.bi_size); + if (is_data_inode(tree->private_data)) - ret = btrfs_submit_data_bio(tree->private_data, bio, mirror_num, + btrfs_submit_data_bio(tree->private_data, bio, mirror_num, bio_flags); else - ret = btrfs_submit_metadata_bio(tree->private_data, bio, + btrfs_submit_metadata_bio(tree->private_data, bio, mirror_num, bio_flags); - - return blk_status_to_errno(ret); + /* + * Above submission hooks will handle the error by ending the bio, + * which will do the cleanup properly. + * So here we should not return any error, or the caller of + * submit_extent_page() will do cleanup again, causing problems. + */ } /* Cleanup unsubmitted bios */ @@ -204,13 +207,12 @@ static void end_write_bio(struct extent_page_data *epd, int ret) * Return 0 if everything is OK. * Return <0 for error. */ -static int __must_check flush_write_bio(struct extent_page_data *epd) +static void flush_write_bio(struct extent_page_data *epd) { - int ret = 0; struct bio *bio = epd->bio_ctrl.bio; if (bio) { - ret = submit_one_bio(bio, 0, 0); + submit_one_bio(bio, 0, 0); /* * Clean up of epd->bio is handled by its endio function. * And endio is either triggered by successful bio execution @@ -220,7 +222,6 @@ static int __must_check flush_write_bio(struct extent_page_data *epd) */ epd->bio_ctrl.bio = NULL; } - return ret; } int __init extent_state_cache_init(void) @@ -3441,10 +3442,8 @@ static int submit_extent_page(unsigned int opf, ASSERT(pg_offset < PAGE_SIZE && size <= PAGE_SIZE && pg_offset + size <= PAGE_SIZE); if (force_bio_submit && bio_ctrl->bio) { - ret = submit_one_bio(bio_ctrl->bio, mirror_num, bio_ctrl->bio_flags); + submit_one_bio(bio_ctrl->bio, mirror_num, bio_ctrl->bio_flags); bio_ctrl->bio = NULL; - if (ret < 0) - return ret; } while (cur < pg_offset + size) { @@ -3485,11 +3484,9 @@ static int submit_extent_page(unsigned int opf, if (added < size - offset) { /* The bio should contain some page(s) */ ASSERT(bio_ctrl->bio->bi_iter.bi_size); - ret = submit_one_bio(bio_ctrl->bio, mirror_num, + submit_one_bio(bio_ctrl->bio, mirror_num, bio_ctrl->bio_flags); bio_ctrl->bio = NULL; - if (ret < 0) - return ret; } cur += added; } @@ -4237,14 +4234,12 @@ static noinline_for_stack int lock_extent_buffer_for_io(struct extent_buffer *eb struct extent_page_data *epd) { struct btrfs_fs_info *fs_info = eb->fs_info; - int i, num_pages, failed_page_nr; + int i, num_pages; int flush = 0; int ret = 0; if (!btrfs_try_tree_write_lock(eb)) { - ret = flush_write_bio(epd); - if (ret < 0) - return ret; + flush_write_bio(epd); flush = 1; btrfs_tree_lock(eb); } @@ -4254,9 +4249,7 @@ static noinline_for_stack int lock_extent_buffer_for_io(struct extent_buffer *eb if (!epd->sync_io) return 0; if (!flush) { - ret = flush_write_bio(epd); - if (ret < 0) - return ret; + flush_write_bio(epd); flush = 1; } while (1) { @@ -4303,39 +4296,13 @@ static noinline_for_stack int lock_extent_buffer_for_io(struct extent_buffer *eb if (!trylock_page(p)) { if (!flush) { - int err; - - err = flush_write_bio(epd); - if (err < 0) { - ret = err; - failed_page_nr = i; - goto err_unlock; - } + flush_write_bio(epd); flush = 1; } lock_page(p); } } - return ret; -err_unlock: - /* Unlock already locked pages */ - for (i = 0; i < failed_page_nr; i++) - unlock_page(eb->pages[i]); - /* - * Clear EXTENT_BUFFER_WRITEBACK and wake up anyone waiting on it. - * Also set back EXTENT_BUFFER_DIRTY so future attempts to this eb can - * be made and undo everything done before. - */ - btrfs_tree_lock(eb); - spin_lock(&eb->refs_lock); - set_bit(EXTENT_BUFFER_DIRTY, &eb->bflags); - end_extent_buffer_writeback(eb); - spin_unlock(&eb->refs_lock); - percpu_counter_add_batch(&fs_info->dirty_metadata_bytes, eb->len, - fs_info->dirty_metadata_batch); - btrfs_clear_header_flag(eb, BTRFS_HEADER_FLAG_WRITTEN); - btrfs_tree_unlock(eb); return ret; } @@ -4957,13 +4924,19 @@ int btree_write_cache_pages(struct address_space *mapping, * if the fs already has error. */ if (!BTRFS_FS_ERROR(fs_info)) { - ret = flush_write_bio(&epd); + flush_write_bio(&epd); } else { ret = -EROFS; end_write_bio(&epd, ret); } out: btrfs_zoned_meta_io_unlock(fs_info); + /* + * We can get ret > 0 from submit_extent_page() indicating how many ebs + * were submitted. Reset it to 0 to avoid false alerts for the caller. + */ + if (ret > 0) + ret = 0; return ret; } @@ -5065,8 +5038,7 @@ static int extent_write_cache_pages(struct address_space *mapping, * tmpfs file mapping */ if (!trylock_page(page)) { - ret = flush_write_bio(epd); - BUG_ON(ret < 0); + flush_write_bio(epd); lock_page(page); } @@ -5076,10 +5048,8 @@ static int extent_write_cache_pages(struct address_space *mapping, } if (wbc->sync_mode != WB_SYNC_NONE) { - if (PageWriteback(page)) { - ret = flush_write_bio(epd); - BUG_ON(ret < 0); - } + if (PageWriteback(page)) + flush_write_bio(epd); wait_on_page_writeback(page); } @@ -5119,9 +5089,8 @@ static int extent_write_cache_pages(struct address_space *mapping, * page in our current bio, and thus deadlock, so flush the * write bio here. */ - ret = flush_write_bio(epd); - if (!ret) - goto retry; + flush_write_bio(epd); + goto retry; } if (wbc->range_cyclic || (wbc->nr_to_write > 0 && range_whole)) @@ -5147,8 +5116,7 @@ int extent_write_full_page(struct page *page, struct writeback_control *wbc) return ret; } - ret = flush_write_bio(&epd); - ASSERT(ret <= 0); + flush_write_bio(&epd); return ret; } @@ -5210,7 +5178,7 @@ int extent_write_locked_range(struct inode *inode, u64 start, u64 end) } if (!found_error) - ret = flush_write_bio(&epd); + flush_write_bio(&epd); else end_write_bio(&epd, ret); @@ -5243,7 +5211,7 @@ int extent_writepages(struct address_space *mapping, end_write_bio(&epd, ret); return ret; } - ret = flush_write_bio(&epd); + flush_write_bio(&epd); return ret; } @@ -5266,10 +5234,8 @@ void extent_readahead(struct readahead_control *rac) if (em_cached) free_extent_map(em_cached); - if (bio_ctrl.bio) { - if (submit_one_bio(bio_ctrl.bio, 0, bio_ctrl.bio_flags)) - return; - } + if (bio_ctrl.bio) + submit_one_bio(bio_ctrl.bio, 0, bio_ctrl.bio_flags); } /* @@ -6649,12 +6615,8 @@ static int read_extent_buffer_subpage(struct extent_buffer *eb, int wait, atomic_dec(&eb->io_pages); } if (bio_ctrl.bio) { - int tmp; - - tmp = submit_one_bio(bio_ctrl.bio, mirror_num, 0); + submit_one_bio(bio_ctrl.bio, mirror_num, 0); bio_ctrl.bio = NULL; - if (tmp < 0) - return tmp; } if (ret || wait != WAIT_COMPLETE) return ret; @@ -6767,10 +6729,8 @@ int read_extent_buffer_pages(struct extent_buffer *eb, int wait, int mirror_num) } if (bio_ctrl.bio) { - err = submit_one_bio(bio_ctrl.bio, mirror_num, bio_ctrl.bio_flags); + submit_one_bio(bio_ctrl.bio, mirror_num, bio_ctrl.bio_flags); bio_ctrl.bio = NULL; - if (err) - return err; } if (ret || wait != WAIT_COMPLETE) diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h index 05253612ce7b..9a283b2358b8 100644 --- a/fs/btrfs/extent_io.h +++ b/fs/btrfs/extent_io.h @@ -178,8 +178,7 @@ typedef struct extent_map *(get_extent_t)(struct btrfs_inode *inode, int try_release_extent_mapping(struct page *page, gfp_t mask); int try_release_extent_buffer(struct page *page); -int __must_check submit_one_bio(struct bio *bio, int mirror_num, - unsigned long bio_flags); +void submit_one_bio(struct bio *bio, int mirror_num, unsigned long bio_flags); int btrfs_do_readpage(struct page *page, struct extent_map **em_cached, struct btrfs_bio_ctrl *bio_ctrl, unsigned int read_flags, u64 *prev_em_start); diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 03fa33db1640..62fdcb7576ba 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -8158,13 +8158,12 @@ int btrfs_readpage(struct file *file, struct page *page) btrfs_lock_and_flush_ordered_range(inode, start, end, NULL); ret = btrfs_do_readpage(page, NULL, &bio_ctrl, 0, NULL); - if (bio_ctrl.bio) { - int ret2; - - ret2 = submit_one_bio(bio_ctrl.bio, 0, bio_ctrl.bio_flags); - if (ret == 0) - ret = ret2; - } + /* + * If btrfs_do_readpage() failed we will want to submit the assembled + * bio to do the cleanup. + */ + if (bio_ctrl.bio) + submit_one_bio(bio_ctrl.bio, 0, bio_ctrl.bio_flags); return ret; } -- 2.35.1

3 years

4
9
0 0

[PATCH net] virtio_net: fix wrong buf address calculation when using xdp

by Nikolay Aleksandrov

We received a report[1] of kernel crashes when Cilium is used in XDP mode with virtio_net after updating to newer kernels. After investigating the reason it turned out that when using mergeable bufs with an XDP program which adjusts xdp.data or xdp.data_meta page_to_buf() calculates the build_skb address wrong because the offset can become less than the headroom so it gets the address of the previous page (-X bytes depending on how lower offset is): page_to_skb: page addr ffff9eb2923e2000 buf ffff9eb2923e1ffc offset 252 headroom 256 This is a pr_err() I added in the beginning of page_to_skb which clearly shows offset that is less than headroom by adding 4 bytes of metadata via an xdp prog. The calculations done are: receive_mergeable(): headroom = VIRTIO_XDP_HEADROOM; // VIRTIO_XDP_HEADROOM == 256 bytes offset = xdp.data - page_address(xdp_page) - vi->hdr_len - metasize; page_to_skb(): p = page_address(page) + offset; ... buf = p - headroom; Now buf goes -4 bytes from the page's starting address as can be seen above which is set as skb->head and skb->data by build_skb later. Depending on what's done with the skb (when it's freed most often) we get all kinds of corruptions and BUG_ON() triggers in mm[2]. The story of the faulty commit is interesting because the patch was sent and applied twice (it seems the first one got lost during merge back in 5.13 window). The first version of the patch that was applied as: commit 7bf64460e3b2 ("virtio-net: get build_skb() buf by data ptr") was actually correct because it calculated the page starting address without relying on offset or headroom, but then the second version that was applied as: commit 8fb7da9e9907 ("virtio_net: get build_skb() buf by data ptr") was wrong and added the above calculation. An example xdp prog[3] is below. [1] https://github.com/cilium/cilium/issues/19453 [2] Two of the many traces: [ 40.437400] BUG: Bad page state in process swapper/0 pfn:14940 [ 40.916726] BUG: Bad page state in process systemd-resolve pfn:053b7 [ 41.300891] kernel BUG at include/linux/mm.h:720! [ 41.301801] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI [ 41.302784] CPU: 1 PID: 1181 Comm: kubelet Kdump: loaded Tainted: G B W 5.18.0-rc1+ #37 [ 41.304458] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15.0-1.fc35 04/01/2014 [ 41.306018] RIP: 0010:page_frag_free+0x79/0xe0 [ 41.306836] Code: 00 00 75 ea 48 8b 07 a9 00 00 01 00 74 e0 48 8b 47 48 48 8d 50 ff a8 01 48 0f 45 fa eb d0 48 c7 c6 18 b8 30 a6 e8 d7 f8 fc ff <0f> 0b 48 8d 78 ff eb bc 48 8b 07 a9 00 00 01 00 74 3a 66 90 0f b6 [ 41.310235] RSP: 0018:ffffac05c2a6bc78 EFLAGS: 00010292 [ 41.311201] RAX: 000000000000003e RBX: 0000000000000000 RCX: 0000000000000000 [ 41.312502] RDX: 0000000000000001 RSI: ffffffffa6423004 RDI: 00000000ffffffff [ 41.313794] RBP: ffff993c98823600 R08: 0000000000000000 R09: 00000000ffffdfff [ 41.315089] R10: ffffac05c2a6ba68 R11: ffffffffa698ca28 R12: ffff993c98823600 [ 41.316398] R13: ffff993c86311ebc R14: 0000000000000000 R15: 000000000000005c [ 41.317700] FS: 00007fe13fc56740(0000) GS:ffff993cdd900000(0000) knlGS:0000000000000000 [ 41.319150] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 41.320152] CR2: 000000c00008a000 CR3: 0000000014908000 CR4: 0000000000350ee0 [ 41.321387] Call Trace: [ 41.321819] <TASK> [ 41.322193] skb_release_data+0x13f/0x1c0 [ 41.322902] __kfree_skb+0x20/0x30 [ 41.343870] tcp_recvmsg_locked+0x671/0x880 [ 41.363764] tcp_recvmsg+0x5e/0x1c0 [ 41.384102] inet_recvmsg+0x42/0x100 [ 41.406783] ? sock_recvmsg+0x1d/0x70 [ 41.428201] sock_read_iter+0x84/0xd0 [ 41.445592] ? 0xffffffffa3000000 [ 41.462442] new_sync_read+0x148/0x160 [ 41.479314] ? 0xffffffffa3000000 [ 41.496937] vfs_read+0x138/0x190 [ 41.517198] ksys_read+0x87/0xc0 [ 41.535336] do_syscall_64+0x3b/0x90 [ 41.551637] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 41.568050] RIP: 0033:0x48765b [ 41.583955] Code: e8 4a 35 fe ff eb 88 cc cc cc cc cc cc cc cc e8 fb 7a fe ff 48 8b 7c 24 10 48 8b 74 24 18 48 8b 54 24 20 48 8b 44 24 08 0f 05 <48> 3d 01 f0 ff ff 76 20 48 c7 44 24 28 ff ff ff ff 48 c7 44 24 30 [ 41.632818] RSP: 002b:000000c000a2f5b8 EFLAGS: 00000212 ORIG_RAX: 0000000000000000 [ 41.664588] RAX: ffffffffffffffda RBX: 000000c000062000 RCX: 000000000048765b [ 41.681205] RDX: 0000000000005e54 RSI: 000000c000e66000 RDI: 0000000000000016 [ 41.697164] RBP: 000000c000a2f608 R08: 0000000000000001 R09: 00000000000001b4 [ 41.713034] R10: 00000000000000b6 R11: 0000000000000212 R12: 00000000000000e9 [ 41.728755] R13: 0000000000000001 R14: 000000c000a92000 R15: ffffffffffffffff [ 41.744254] </TASK> [ 41.758585] Modules linked in: br_netfilter bridge veth netconsole virtio_net and [ 33.524802] BUG: Bad page state in process systemd-network pfn:11e60 [ 33.528617] page ffffe05dc0147b00 ffffe05dc04e7a00 ffff8ae9851ec000 (1) len 82 offset 252 metasize 4 hroom 0 hdr_len 12 data ffff8ae9851ec10c data_meta ffff8ae9851ec108 data_end ffff8ae9851ec14e [ 33.529764] page:000000003792b5ba refcount:0 mapcount:-512 mapping:0000000000000000 index:0x0 pfn:0x11e60 [ 33.532463] flags: 0xfffffc0000000(node=0|zone=1|lastcpupid=0x1fffff) [ 33.532468] raw: 000fffffc0000000 0000000000000000 dead000000000122 0000000000000000 [ 33.532470] raw: 0000000000000000 0000000000000000 00000000fffffdff 0000000000000000 [ 33.532471] page dumped because: nonzero mapcount [ 33.532472] Modules linked in: br_netfilter bridge veth netconsole virtio_net [ 33.532479] CPU: 0 PID: 791 Comm: systemd-network Kdump: loaded Not tainted 5.18.0-rc1+ #37 [ 33.532482] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15.0-1.fc35 04/01/2014 [ 33.532484] Call Trace: [ 33.532496] <TASK> [ 33.532500] dump_stack_lvl+0x45/0x5a [ 33.532506] bad_page.cold+0x63/0x94 [ 33.532510] free_pcp_prepare+0x290/0x420 [ 33.532515] free_unref_page+0x1b/0x100 [ 33.532518] skb_release_data+0x13f/0x1c0 [ 33.532524] kfree_skb_reason+0x3e/0xc0 [ 33.532527] ip6_mc_input+0x23c/0x2b0 [ 33.532531] ip6_sublist_rcv_finish+0x83/0x90 [ 33.532534] ip6_sublist_rcv+0x22b/0x2b0 [3] XDP program to reproduce(xdp_pass.c): #include <linux/bpf.h> #include <bpf/bpf_helpers.h> SEC("xdp_pass") int xdp_pkt_pass(struct xdp_md *ctx) { bpf_xdp_adjust_head(ctx, -(int)32); return XDP_PASS; } char _license[] SEC("license") = "GPL"; compile: clang -O2 -g -Wall -target bpf -c xdp_pass.c -o xdp_pass.o load on virtio_net: ip link set enp1s0 xdpdrv obj xdp_pass.o sec xdp_pass CC: stable(a)vger.kernel.org CC: Jason Wang <jasowang(a)redhat.com> CC: Xuan Zhuo <xuanzhuo(a)linux.alibaba.com> CC: Daniel Borkmann <daniel(a)iogearbox.net> CC: "Michael S. Tsirkin" <mst(a)redhat.com> CC: virtualization(a)lists.linux-foundation.org Fixes: 8fb7da9e9907 ("virtio_net: get build_skb() buf by data ptr") Signed-off-by: Nikolay Aleksandrov <razor(a)blackwall.org> --- drivers/net/virtio_net.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 87838cbe38cf..0687dd88e97f 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -434,9 +434,13 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, * Buffers with headroom use PAGE_SIZE as alloc size, see * add_recvbuf_mergeable() + get_mergeable_buf_len() */ - truesize = headroom ? PAGE_SIZE : truesize; + if (headroom) { + truesize = PAGE_SIZE; + buf = (char *)((unsigned long)p & PAGE_MASK); + } else { + buf = p; + } tailroom = truesize - headroom; - buf = p - headroom; len -= hdr_len; offset += hdr_padded_len; -- 2.35.1

3 years

2
17
0 0

[git:media_stage/master] media: coda: Add more H264 levels for CODA960

by Mauro Carvalho Chehab

This is an automatic generated email to let you know that the following patch were queued: Subject: media: coda: Add more H264 levels for CODA960 Author: Nicolas Dufresne <nicolas.dufresne(a)collabora.com> Date: Wed Apr 6 21:23:43 2022 +0100 Add H264 level 1.0, 4.1, 4.2 to the list of supported formats. While the hardware does not fully support these levels, it does support most of them. The constraints on frame size and pixel formats already cover the limitation. This fixes negotiation of level on GStreamer 1.17.1. Cc: stable(a)vger.kernel.org Fixes: 42a68012e67c2 ("media: coda: add read-only h.264 decoder profile/level controls") Suggested-by: Philipp Zabel <p.zabel(a)pengutronix.de> Signed-off-by: Nicolas Dufresne <nicolas.dufresne(a)collabora.com> Signed-off-by: Ezequiel Garcia <ezequiel(a)collabora.com> Signed-off-by: Fabio Estevam <festevam(a)denx.de> Reviewed-by: Philipp Zabel <p.zabel(a)pengutronix.de> Signed-off-by: Hans Verkuil <hverkuil-cisco(a)xs4all.nl> Signed-off-by: Mauro Carvalho Chehab <mchehab(a)kernel.org> drivers/media/platform/chips-media/coda-common.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) --- diff --git a/drivers/media/platform/chips-media/coda-common.c b/drivers/media/platform/chips-media/coda-common.c index 36ec5a50a491..d246afcb3f49 100644 --- a/drivers/media/platform/chips-media/coda-common.c +++ b/drivers/media/platform/chips-media/coda-common.c @@ -2347,12 +2347,15 @@ static void coda_encode_ctrls(struct coda_ctx *ctx) if (ctx->dev->devtype->product == CODA_960) { v4l2_ctrl_new_std_menu(&ctx->ctrls, &coda_ctrl_ops, V4L2_CID_MPEG_VIDEO_H264_LEVEL, - V4L2_MPEG_VIDEO_H264_LEVEL_4_0, - ~((1 << V4L2_MPEG_VIDEO_H264_LEVEL_2_0) | + V4L2_MPEG_VIDEO_H264_LEVEL_4_2, + ~((1 << V4L2_MPEG_VIDEO_H264_LEVEL_1_0) | + (1 << V4L2_MPEG_VIDEO_H264_LEVEL_2_0) | (1 << V4L2_MPEG_VIDEO_H264_LEVEL_3_0) | (1 << V4L2_MPEG_VIDEO_H264_LEVEL_3_1) | (1 << V4L2_MPEG_VIDEO_H264_LEVEL_3_2) | - (1 << V4L2_MPEG_VIDEO_H264_LEVEL_4_0)), + (1 << V4L2_MPEG_VIDEO_H264_LEVEL_4_0) | + (1 << V4L2_MPEG_VIDEO_H264_LEVEL_4_1) | + (1 << V4L2_MPEG_VIDEO_H264_LEVEL_4_2)), V4L2_MPEG_VIDEO_H264_LEVEL_4_0); } v4l2_ctrl_new_std(&ctx->ctrls, &coda_ctrl_ops,

3 years

1
0
0 0

[git:media_stage/master] media: coda: Fix reported H264 profile

by Mauro Carvalho Chehab

This is an automatic generated email to let you know that the following patch were queued: Subject: media: coda: Fix reported H264 profile Author: Nicolas Dufresne <nicolas.dufresne(a)collabora.com> Date: Wed Apr 6 21:23:42 2022 +0100 The CODA960 manual states that ASO/FMO features of baseline are not supported, so for this reason this driver should only report constrained baseline support. This fixes negotiation issue with constrained baseline content on GStreamer 1.17.1. ASO/FMO features are unsupported for the encoder and untested for the decoder because there is currently no userspace support. Neither GStreamer parsers nor FFMPEG parsers support ASO/FMO. Cc: stable(a)vger.kernel.org Fixes: 42a68012e67c2 ("media: coda: add read-only h.264 decoder profile/level controls") Signed-off-by: Nicolas Dufresne <nicolas.dufresne(a)collabora.com> Signed-off-by: Ezequiel Garcia <ezequiel(a)collabora.com> Tested-by: Pascal Speck <kernel(a)iktek.de> Signed-off-by: Fabio Estevam <festevam(a)denx.de> Reviewed-by: Philipp Zabel <p.zabel(a)pengutronix.de> Signed-off-by: Hans Verkuil <hverkuil-cisco(a)xs4all.nl> Signed-off-by: Mauro Carvalho Chehab <mchehab(a)kernel.org> drivers/media/platform/chips-media/coda-common.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) --- diff --git a/drivers/media/platform/chips-media/coda-common.c b/drivers/media/platform/chips-media/coda-common.c index 7b4942bb6c2c..36ec5a50a491 100644 --- a/drivers/media/platform/chips-media/coda-common.c +++ b/drivers/media/platform/chips-media/coda-common.c @@ -2332,8 +2332,8 @@ static void coda_encode_ctrls(struct coda_ctx *ctx) V4L2_CID_MPEG_VIDEO_H264_CHROMA_QP_INDEX_OFFSET, -12, 12, 1, 0); v4l2_ctrl_new_std_menu(&ctx->ctrls, &coda_ctrl_ops, V4L2_CID_MPEG_VIDEO_H264_PROFILE, - V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE, 0x0, - V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE); + V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE, 0x0, + V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE); if (ctx->dev->devtype->product == CODA_HX4 || ctx->dev->devtype->product == CODA_7541) { v4l2_ctrl_new_std_menu(&ctx->ctrls, &coda_ctrl_ops, @@ -2414,7 +2414,7 @@ static void coda_decode_ctrls(struct coda_ctx *ctx) ctx->h264_profile_ctrl = v4l2_ctrl_new_std_menu(&ctx->ctrls, &coda_ctrl_ops, V4L2_CID_MPEG_VIDEO_H264_PROFILE, V4L2_MPEG_VIDEO_H264_PROFILE_HIGH, - ~((1 << V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) | + ~((1 << V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE) | (1 << V4L2_MPEG_VIDEO_H264_PROFILE_MAIN) | (1 << V4L2_MPEG_VIDEO_H264_PROFILE_HIGH)), V4L2_MPEG_VIDEO_H264_PROFILE_HIGH);

3 years

1
0
0 0

[PATCH V3] riscv: patch_text: Fixup last cpu should be master

by guoren＠kernel.org

From: Guo Ren <guoren(a)linux.alibaba.com> These patch_text implementations are using stop_machine_cpuslocked infrastructure with atomic cpu_count. The original idea: When the master CPU patch_text, the others should wait for it. But current implementation is using the first CPU as master, which couldn't guarantee the remaining CPUs are waiting. This patch changes the last CPU as the master to solve the potential risk. Signed-off-by: Guo Ren <guoren(a)linux.alibaba.com> Signed-off-by: Guo Ren <guoren(a)kernel.org> Acked-by: Palmer Dabbelt <palmer(a)rivosinc.com> Reviewed-by: Masami Hiramatsu <mhiramat(a)kernel.org> Cc: <stable(a)vger.kernel.org> --- arch/riscv/kernel/patch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c index 0b552873a577..765004b60513 100644 --- a/arch/riscv/kernel/patch.c +++ b/arch/riscv/kernel/patch.c @@ -104,7 +104,7 @@ static int patch_text_cb(void *data) struct patch_insn *patch = data; int ret = 0; - if (atomic_inc_return(&patch->cpu_count) == 1) { + if (atomic_inc_return(&patch->cpu_count) == num_online_cpus()) { ret = patch_text_nosync(patch->addr, &patch->insn, GET_INSN_LENGTH(patch->insn)); -- 2.25.1

3 years

5
7
0 0

Re: Fix a devlink AB-BA deadlock on net namespace deletion

by gregkh

On Sun, Apr 24, 2022 at 02:02:22PM +0800, 张广辉 wrote: > > Hi all > <snip> Hi, This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree. You are receiving this message because of the following common error(s) as indicated below: - Your patch is malformed (tabs converted to spaces, linewrapped, etc.) and can not be applied. Please read the file, Documentation/email-clients.txt in order to fix this. If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers. thanks, greg k-h's patch email bot

3 years

1
0
0 0

Good Day,

by Mr. Jibri loubda

-- Greetings, I'm Mr. Jibri loubda, how are you doing hope you are in good health, the Board irector try to reach you on phone several times Meanwhile, your number was not connecting. before he ask me to send you an email to hear from you if you are fine. hope to hear you are in good Health. Thanks, Mr. Jibri loubda.

3 years

1
0
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror April 2022