| Age | Commit message (Collapse) | Author |
|---|
|
__ocfs2_journal_access() refuses to journal a buffer whose previous write
failed, and turns the filesystem read-only rather than risk metadata
inconsistency. That check sits inside an if (!buffer_uptodate(bh)) block,
because until now a failed write also cleared BH_Uptodate.
This series stops clearing BH_Uptodate on write error, so that outer test
would never fire again and ocfs2 would silently start reusing buffers whose
last write failed. Hoist the check out of the debug block, where it does
not depend on BH_Uptodate any more, and drop the now dead second half of
its condition.
The mlog() pair keeps its own !buffer_uptodate() guard: it is a separate
"we can safely remove this assertion after testing" debug aid about being
handed a buffer with no valid contents, which is a different question from
whether the last write of that buffer failed.
The unlocked test followed by a locked retest is deliberate. BH_Write_EIO
is cleared under the buffer lock, so taking the lock and looking a second
time avoids turning the
filesystem read-only over an error that a concurrent rewrite has already
cleared, while keeping the common case lock-free.
The code in this patch is Jan's, from the review discussion linked in the
cover letter.
Suggested-by: Jan Kara <jack@suse.cz>
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/ad75c40927060cafe5a8ac45ae71c02ae4dfba04.1785951556.git.coshi036@gmail.com
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
ocfs2_write_block() and ocfs2_write_super_or_backup() detect a failed write
by looking at BH_Uptodate afterwards. That relies on the write completion
handler clearing BH_Uptodate on error, which this series removes: a buffer
whose write failed still holds the data the filesystem asked to be written,
so declaring it not up to date is wrong and makes callers re-read it.
Test BH_Write_EIO instead. Note that ocfs2_write_block()'s test is the
positive one, so the sense has to be inverted rather than the flag simply
swapped.
The comment in ocfs2_write_block()'s error arm needs updating for the same
reason. It said the clustered uptodate information did not have to be
removed because the buffer was not marked locally uptodate; after this
series it is, so the reason no longer holds. Not advertising the block to
the cluster is still the right thing to do - the data is in memory but not
on disk - so only the justification changes, not the behaviour.
No behaviour change today - a failed write sets BH_Write_EIO and clears
BH_Uptodate together. It stops being a no-op at the end of the series,
where the new test is the one that still works.
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/05cd3c34f2dc69b542db7baaef058cd75005d4cb.1785951556.git.coshi036@gmail.com
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Two places detect a failed metadata write by testing !buffer_uptodate()
after waiting for it. That relies on the write completion handler clearing
BH_Uptodate on error, which this series removes: a buffer whose write
failed still holds the data the filesystem asked to be written, so
declaring it not up to date is wrong and makes callers re-read it.
ext4 already does this correctly for the superblock - see
ext4_commit_super(), which tests buffer_write_io_error() - so this brings
the other two into line.
In __ext4_handle_dirty_metadata() the old test also required BH_Req.
BH_Write_EIO implies it, so the pair collapses into one test. The new test
is also strictly stronger than consuming sync_dirty_buffer()'s return
value, because it still fires when the buffer was written by background
writeback and that write hit an error, which sync_dirty_buffer() does not
report.
Note that the failing write does not clear BH_Write_EIO, so an unrepaired
itable block now reports on every subsequent sync of that inode rather than
only on the write that failed. That is the intended behaviour, and matches
what ocfs2 has always done with this flag.
No behaviour change today - a failed write sets BH_Write_EIO and clears
BH_Uptodate together. It stops being a no-op at the end of the series,
where the new test is the one that still works.
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/568e57d184da17041872fd4b498f31dd6259a4c1.1785951556.git.coshi036@gmail.com
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
fat_sync_bhs() waits for the writes it issued and then tests
!buffer_uptodate() to find the ones that failed. That relies on the write
completion handler clearing BH_Uptodate on error, which this series
removes: a buffer whose write failed still holds the data the filesystem
asked to be written, so declaring it not up to date is wrong and makes
callers re-read it.
Test BH_Write_EIO, which is what the completion handler sets and what this
code actually wants to know.
No behaviour change today - a failed write sets BH_Write_EIO and clears
BH_Uptodate together. It stops being a no-op at the end of the series,
where the new test is the one that still works.
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
Link: https://patch.msgid.link/4b6019b5a48b83c8235918b084248983a57692e1.1785951556.git.coshi036@gmail.com
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
exfat_update_bhs() waits for the writes it issued and then tests
!buffer_uptodate() to find the ones that failed. That relies on the write
completion handler clearing BH_Uptodate on error, which this series
removes: a buffer whose write failed still holds the data the filesystem
asked to be written, so declaring it not up to date is wrong and makes
callers re-read it.
Test BH_Write_EIO, which is what the completion handler sets and what this
code actually wants to know.
No behaviour change today - a failed write sets BH_Write_EIO and clears
BH_Uptodate together. It stops being a no-op at the end of the series,
where the new test is the one that still works.
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
Link: https://patch.msgid.link/0784ef63a525434e7c0aff730eca7b43e043095d.1785951556.git.coshi036@gmail.com
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
__omfs_write_inode() spots a failed synchronous write, on both the primary
block and each mirror, by testing BH_Req together with !BH_Uptodate. That
relies on the write completion handler clearing BH_Uptodate on error, which
this series removes: a buffer whose write failed still holds the data the
filesystem asked to be written, so declaring it not up to date is wrong and
makes callers re-read it.
BH_Write_EIO says exactly what this code wants to know, and it implies
BH_Req, so each pair collapses into one test.
No behaviour change today - a failed write sets BH_Write_EIO and clears
BH_Uptodate together. It stops being a no-op at the end of the series,
where the new test is the one that still works.
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
Link: https://patch.msgid.link/b33ccb12c29ae743ccfa48cdf1e9140fbf2dabcc.1785951556.git.coshi036@gmail.com
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
ext2_xattr_set2() spots a failed synchronous write by testing BH_Req
together with !BH_Uptodate. That relies on the write completion handler
clearing BH_Uptodate on error, which this series removes: a buffer whose
write failed still holds the data the filesystem asked to be written, so
declaring it not up to date is wrong and makes callers re-read it.
BH_Write_EIO says exactly what this code wants to know, and it implies
BH_Req, so the pair collapses into one test.
No behaviour change today - a failed write sets BH_Write_EIO and clears
BH_Uptodate together. It stops being a no-op at the end of the series,
where the new test is the one that still works.
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/fcc530fffd0f1e18e8cd1a3974bc0ccc5b4c4b66.1785951556.git.coshi036@gmail.com
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
adfs_dir_sync() spots a failed write by testing BH_Req together with
!BH_Uptodate. That relies on the write completion handler clearing
BH_Uptodate on error, which this series removes: a buffer whose write
failed still holds the data the filesystem asked to be written, so
declaring it not up to date is wrong and makes callers re-read it.
BH_Write_EIO says exactly what this code wants to know, and it implies
BH_Req, so the pair collapses into one test.
No behaviour change today - a failed write sets BH_Write_EIO and clears
BH_Uptodate together. It stops being a no-op at the end of the series,
where the new test is the one that still works.
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
Link: https://patch.msgid.link/05cf8ad911ad7a6eb68f42e38f1eea39c38e246b.1785951556.git.coshi036@gmail.com
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Both places in this file that report a metadata write error to a caller do
it by testing !buffer_uptodate() after waiting for the write. That works
only because the write completion handlers clear BH_Uptodate when the write
fails, which is what this series is removing: a buffer whose write failed
still holds the correct data, and saying otherwise makes callers rewrite,
re-read or WARN over a buffer that was never wrong.
BH_Write_EIO is the flag that actually means "the last write of this buffer
failed", and both handlers already set it via mark_buffer_write_io_error().
Test that instead.
No behaviour change: today a failed write through bh_end_write() or
bh_end_async_write() sets BH_Write_EIO and clears BH_Uptodate together, so
the two tests agree. They stop agreeing at the end of the series, and this
one stays right.
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/2b309196b883cc8979800911a47668e225401b2c.1785951556.git.coshi036@gmail.com
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
discard_buffer() strips the state that describes where a buffer lives and
what has happened to it, because after an invalidate none of it applies any
more. BH_Write_EIO belongs in that set for the same reason: it describes a
write of the data that is being thrown away.
Leaving it set means a buffer_head reused for a different block starts life
carrying somebody else's write error. Like the bforget() change, this is
mostly theoretical today and becomes load bearing once the rest of the
series makes BH_Write_EIO the report of a failed metadata write.
Suggested-by: Jan Kara <jack@suse.cz>
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/c6e9db48d8d0feb83d4ca29306f4bc1e58f1ee0f.1785951556.git.coshi036@gmail.com
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
BH_Write_EIO records that the last write of this buffer failed. It is
cleared when the buffer is written again, but a filesystem freeing a
metadata block never writes it again. It calls bforget() and hands the
block back to the allocator, so the flag outlives the block it refers to.
That does not matter much today, because the write error is also recorded
by clearing BH_Uptodate and the buffer is discarded soon after. It starts
to matter in the rest of this series, which stops clearing BH_Uptodate on
write error and makes BH_Write_EIO the way a failed metadata write is
reported.
bforget() is where a filesystem says it no longer cares about this
buffer's contents, so clear the error there alongside the dirty flag.
Suggested-by: Jan Kara <jack@suse.cz>
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/ebe0b4f179ccdac7a9400fe2611623ce218c8d87.1785951556.git.coshi036@gmail.com
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
folio_mapping() was doing two jobs here. One was to turn a slab folio into
NULL, which is what made this safe for jbd2's shadow buffers; the previous
patch removed the need for that by giving those buffers no folio at all.
The other is a hazard. folio_mapping() maps a folio in the swap cache to
its swap_address_space, so if a buffer_head were ever attached to such a
folio this would hand fscrypt a swap mapping and dereference ->host on it.
There is no reason to want that here: this path wants the file's mapping or
nothing.
Read ->mapping directly. Buffers with no folio are already handled above.
Suggested-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
Link: https://patch.msgid.link/3fe72ec37bf8491a69031db5f3ba1319da935b97.1785951556.git.coshi036@gmail.com
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
When a metadata buffer has to be copied out before it can be journalled,
jbd2_journal_write_metadata_buffer() writes jh->b_frozen_data rather than
the page cache copy. b_frozen_data is kmalloc()ed, so folio_set_bh() makes
the shadow buffer point at a slab folio.
That is not something the buffer_head layer can reason about. A slab folio
overloads ->mapping, so a shadow buffer looks like it belongs to an
address_space when it does not. buffer_set_crypto_ctx() already has to
work around this, and it is the reason mark_buffer_write_io_error() cannot
be called on a shadow buffer today.
Point the shadow buffer at the frozen data itself instead: leave b_folio
NULL, which it already is out of alloc_buffer_head(), and set b_data. The
previous patch taught fs/buffer.c to submit such a buffer. folio_set_bh()
is now needed on only one path - the one that journals the page cache copy
directly - so it moves there, and new_folio, new_offset and the flag that
used to pick between them all go away.
The two commit-path checksum helpers reach the shadow buffer's contents
through a new kmap_local_bh()/kunmap_local_bh() pair, which handle a buffer
with or without a folio. Memory outside the page cache is always mapped,
so for those there is nothing to map or unmap. Mapping it anyway would be
worse than pointless: with CONFIG_DEBUG_KMAP_LOCAL_FORCE_MAP,
kmap_local_page() hands back a one page mapping even for such memory, which
is not enough for a buffer bigger than a page.
Tested with ext4 mounted data=journal,journal_checksum on a metadata_csum
filesystem, writing files whose every block begins with the JBD2 magic so
that escaping forces the copy-out, then crashing with sysrq-b without
unmounting and replaying the journal on the next mount. Recovery
completed, the file contents matched, e2fsck -fn was clean, and an
instrumented build confirmed the b_folio == NULL path was taken.
Suggested-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
Link: https://patch.msgid.link/6140cd23beb88e99f40eaeff4044a16213f6caab.1785951556.git.coshi036@gmail.com
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
jbd2 builds a temporary buffer_head to write out the frozen copy of a
metadata block, and that copy lives in slab memory. Today jbd2 points the
temporary buffer at the slab folio backing it. A slab folio's ->mapping is
not an address_space, so anything that follows bh->b_folio->mapping there
gets garbage rather than NULL; mark_buffer_write_io_error() does exactly
that, and we are about to start calling it on this buffer.
Rather than teach every such helper about slab folios, allow bh->b_folio to
be NULL and let b_data point straight at the memory. Code that needs the
folio has to check. There are two places in this file:
- __bh_submit() adds the data by virtual address using
bio_add_virt_nofail(), and skips the cgroup accounting: a buffer that is
not in the page cache has no owning folio to attribute writeback to.
- buffer_set_crypto_ctx() returns early. fscrypt has no interest in a
buffer that is not part of a file mapping, which is why it already
returns when the folio has no mapping.
Nothing sets b_folio to NULL yet, so this patch is a no-op on its own.
A buffer_head without a folio is a narrow thing, not a new general
capability. Most of the buffer_head API assumes a folio and will fault or
corrupt state without one - touch_buffer(), bh_offset(), the async read
completion path, and plenty more - so it is up to whoever builds such a
buffer to keep it away from all of that. What NULL buys us is that getting
it wrong fails loudly instead of quietly following a slab folio's
overloaded ->mapping. It is also only valid over memory that is always
mapped: buffers over highmem have no permanent kernel virtual address,
which is why folio_set_bh() records a folio and an offset instead.
Suggested-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/bb6fab2111a48d7ba61887fc1372362576fce6e6.1785951556.git.coshi036@gmail.com
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
All users except bh_offset() have been converted to use b_folio instead.
Convert bh_offset() and remove b_page.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Signed-off-by: Chao Shi <coshi036@gmail.com>
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Reviewed-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/e9d168901578902bffe78ac8b5dfaf1210ee7fb3.1785951556.git.coshi036@gmail.com
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
"WARNING in do_new_mount" saga
ntfs_create_inode() creates a new inode via ntfs_new_inode(). It hashes
it with insert_inode_locked() and so it's marked as I_NEW until
unlock_new_inode().
ntfs 3 calls d_instantiate() in between though... Since the dentry was
already hashed by the lookup before the create any path walk finds it
without touching the parent's i_rwsem and so can lock the inode.
If the inode is a directory unlock_new_inode() calls
lockdep_annotate_inode_mutex_key() and marks i_rwsem with the
i_mutex_dir_key class.
That resets the count and the owner of a lock somebody else may already
hold by now...
syzbot has been spamming us with the same godforsaken bug
"WARNING in do_new_mount"
since 2023. I can't take it anymore so I went looking. Afaict, syzbot's
executor chdirs into a freshly mounted ntfs3 image, creates a
directory and then mounts some pseudofs on it. Everytime the mkdir()
takes longer than syzbot waits mount() runs concurrently:
mkdir("./sys") mount(NULL, "./sys", "sysfs")
ntfs_create_inode()
d_instantiate()
user_path_at() finds the dentry
do_lock_mount()
inode_lock(inode)
namespace_lock()
unlock_new_inode()
lockdep_annotate_inode_mutex_key()
init_rwsem(&inode->i_rwsem)
unlock_mount()
inode_unlock(inode)
The mount side then releases a lock that according to the rwsem nobody
holds:
DEBUG_RWSEMS_WARN_ON((rwsem_owner(sem) != current) && ...):
count = 0x0, magic = 0xffff888043a854e8, owner = 0x0,
curr 0xffff888000244880, list empty
WARNING: CPU: 0 PID: 5346 at kernel/locking/rwsem.c:1368 __up_write
Call Trace:
inode_unlock include/linux/fs.h:877 [inline]
unlock_mount fs/namespace.c:2892 [inline]
do_new_mount_fc fs/namespace.c:3828 [inline]
do_new_mount+0x777/0xa40 fs/namespace.c:3887
On PREEMPT_RT the same thing shows up as
DEBUG_LOCKS_WARN_ON(rt_mutex_owner(lock) != current)
WARNING: kernel/locking/rtmutex_common.h:193 at rt_mutex_slowunlock
The up_write() underflows the reset count. A following inode_lock() on
that directory then never returns. A path walk into the new directory
racing with the mkdir() corrupts the lock the same way via
inode_lock_shared() in lookup_slow().
Switch to d_instantiate_new() and drop the trailing unlock_new_inode().
All error paths bail out before that point with I_NEW still set and
keep using discard_new_inode().
May we never see this fscking bug report again.
Link: https://patch.msgid.link/20260909-work-ntfs3-d_instantiate_new-v1-1-2db697162ce8@kernel.org
Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")
Reviewed-by: Jan Kara <jack@suse.cz>
Cc: stable@vger.kernel.org # v5.15+
Reported-by: syzbot+2a13ad6914e6fcec716c@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/6a9beced.a5e650b3.26d8a.000b.GAE@google.com
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
filesystems/eventfd, filesystems/open_tree_ns and filesystems/xattr were
never added to TARGETS when introduced. filesystems/openat2 was moved
from selftests/openat2/ but the TARGETS entry was never updated, leaving a
stale entry pointing at a directory that no longer exists.
Fix this by adding the four missing subdirectories to TARGETS and
removing the stale openat2 entry.
Link: https://lore.kernel.org/20260703150742.58991-1-disgoel@linux.ibm.com
Fixes: 7c37857fc23a ("selftests: add eventfd selftests")
Fixes: b8f7622aa6e3 ("selftests/open_tree: add OPEN_TREE_NAMESPACE tests")
Fixes: 7e28fef5d4db ("selftests/xattr: path-based AF_UNIX socket xattr tests")
Fixes: fe087927046c ("selftests: move openat2 tests to selftests/filesystems/")
Signed-off-by: Disha Goel <disgoel@linux.ibm.com>
Reviewed-by: Christian Brauner (Amutable) <brauner@kernel.org>
Cc: "Darrick J. Wong" <djwong@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Jeff Layton <jlayton@kernel.org>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Wen Yang <wenyang.linux@foxmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Link: https://patch.msgid.link/20260904183659.B81CD1F00A3D@smtp.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Commit 14b007e17881 added an address check using iter_iov_addr() and a
length check using iter_iov_len() to iov_iter_extract_bvecs(), but these
cannot be used so and are unsafe in this circumstance as the functions have
hardwired assumptions about the iterator type. They should only be used
with ITER_UBUF or ITER_IOVEC-type iterators; they shouldn't be used with
ITER_BVEC, ITER_KVEC, ITER_FOLIOQ, ITER_XARRAY or ITER_DISCARD iterators.
This proves to be a problem for cachefiles as an iterator of type
ITER_FOLIOQ is passed and iter_iov_addr() and iter_iov_len() both
malfunction because iter->__iov in iter_iov() is not pointing to an iovec
array.
Fix this by using iov_iter_alignment() instead.
Fixes: 14b007e17881 ("block: validate user space vectors during extraction")
Signed-off-by: David Howells <dhowells@redhat.com>
Link: https://patch.msgid.link/1667275.1788941191@warthog.procyon.org.uk
Reviewed-by: Keith Busch <kbusch@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
cc: Hannes Reinecke <hare@kernel.org>
cc: Christoph Hellwig <hch@infradead.org>
cc: Jens Axboe <axboe@kernel.dk>
cc: Alexander Viro <viro@zeniv.linux.org.uk>
cc: Paulo Alcantara <pc@manguebit.org>
cc: netfs@lists.linux.dev
cc: linux-block@vger.kernel.org
cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
efi_set_event_at() documents ids although its array index argument is
named idx. This produces missing and excess parameter warnings.
Match the argument name and fix the adjacent spelling of array.
Fixes: 8c0a839c2bcc ("efi/libstub: Descriptions for stub helper functions")
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
|
|
The ring API already supports polling so that once there is anything in
the RX ring, it will mask the ring interrupt and call the ->start_poll
callback. This is the same mechanism that the USB4/Thunderbolt
networking driver already uses.
We can do the same for the USB4STREAM driver and poll any RX frames
while they are available, until we re-enable the ring interrupt again.
This provides better latency when the stream is in "normal"
(non-busy-polling) mode.
Assisted-by: LLM
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
Each time we mask or unmask ring interrupt we do a read-modify-write and
that is not without a cost when on a hot path. We can get rid of that
read by making a shadow copy of the interrupt mask register and using
that in place of a register read.
Assisted-by: LLM
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
nhi_mask_interrupt() and nhi_clear_interrupt() take "ring" as parameter
but in fact it is not an actual ring but a byte offset to the interrupt
register. Make this less confusing and name the paramers what it really
is and calculate the offset where it is actually needed.
In addition ring_interrupt_active() has two variables called "index"
with different meanings, and the second one shadows the first one
open-coding ring_interrupt_index() as well. Drop that and rename the
remaining what they actually hold.
While there, make the mask variable u32 and use BIT() to avoid signed
shifting.
No functional changes intended.
Assisted-by: LLM
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
With ->start_poll it is possible that the ring is actually already
disabled before tb_ring_stop() is called. This is expected behaviour so
there is no point warning about that.
Assisted-by: LLM
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
In adis16400_initial_setup(), the return value of spi_setup() was ignored.
If SPI configuration fails, the driver would proceed with the
initialization anyway, leading to undefined behavior or silent failures
during device setup.
Check the return value of spi_setup() and return the error code immediately
if it fails.
Fixes: cd888a17f95f ("staging:iio:adis16400: Use adis library")
Signed-off-by: Salah Triki <salah.triki@gmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
|
|
The "interrupt polarity high"-macro for KX022A variant is defined as:
"#define KX022A_MASK_IPOL KX022A_MASK_IPOL1"
However, the KX022A_MASK_IPOL1 is not defined anywhere, so actually
using the KX022A_IPOL_HIGH would produce a compile error.
Fix the define by using correct mask.
Fixes: 7c1d1677b322 ("iio: accel: Support Kionix/ROHM KX022A accelerometer")
Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
|
|
Add support for AD5529R 16-channel, 12/16 bit Digital to Analog Converter
from Analog Devices.
The device communicates over SPI and supports per-channel output range
configuration. An optional external 4.096V reference can be used in
place of the internal reference.
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5529r.pdf
Signed-off-by: Janani Sunil <janani.sunil@analog.com>
Signed-off-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
|
|
Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
buffered voltage output digital-to-analog converter (DAC) with an
integrated precision reference.
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Janani Sunil <janani.sunil@analog.com>
Signed-off-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
|
|
Some SPI devices support sharing a single chip select across multiple
physical chips by encoding a device address in the SPI frame itself.
Add the generic spi-device-addr property for describing these hardware
addresses. The property is placed on the SPI peripheral node and may
contain multiple addresses.
Acked-by: Nuno Sá <nuno.sa@analog.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Signed-off-by: Janani Sunil <janani.sunil@analog.com>
Signed-off-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
|
|
When using both accel and gyro with different frequencies, the ODR
change flag in the FIFO header can be sometime in a packet with
invalid data. Since we are ignoring these packets, we miss the ODR
change and any further change are blocked until sensor is turned off.
Fix this by moving the packet data validity check after the ODR change
flag check.
Fixes: ec74ae9fd37c ("iio: imu: inv_icm42600: add accurate timestamping")
Cc: stable@vger.kernel.org
Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
Signed-off-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
|
|
Add support for the AD5710R/AD5711R, 8-channel 16-/12-bit configurable
IDAC/VDAC parts. They share the AD3530R register map and access model,
so fold them into this driver.
Each channel is configured as voltage or current output from its DT
channel@N node via adi,ch-func, building the iio_chan_spec dynamically.
Voltage channels enable VMODE_EN and report the reference-derived scale,
current channels report the 50 mA internal Iref scale. The powerdown
mode is read-only and derived from the channel's configured type.
Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
|
|
In preparation for adding the AD5710R/AD5711R, which support selectable
12-/16-bit resolution, add a per-chip resolution field and use it in the
raw read/write and scale paths instead of assuming 16 bits. Drop
AD3530R_REG_VAL_MASK and AD3530R_DAC_MAX_VAL, which hardcoded the 16-bit
width and are now unused.
Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
|
|
Add device tree bindings for the Analog Devices AD5710R/AD5711R
8-channel 12-/16-bit Configurable IDAC/VDAC.
Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
|
|
Document the out_currentY_powerdown and out_currentY_powerdown_mode
attributes for current-output DAC channels, and add the 15kohm_to_gnd
powerdown mode.
Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
|
|
commit 79104becf42b ("sched/fair: Forfeit vruntime on yield") changed
yield_task_fair() to only bump the deadline when the entity is eligible
(vruntime <= avg_vruntime). When the entity is ineligible the yield
becomes a complete no-op from scheduling perspective.
lost_exception_test calls sched_yield() 100,000 times per iteration to
race the EBB exception delivery with a context switch to the eat_cpu
companion process. After enough iterations the test process's vruntime
races ahead of avg_vruntime (each eligible yield bumps vruntime to
deadline, then advances deadline by one slice). Once ineligible,
yield_task_fair() does nothing: so the scheduler won't pick the
eat_cpu child. No context switch occurs, the PMAO race is never
triggered, and ebb_count stays at 0 forever causing the test to
hang until timeout.
Fix by replacing sched_yield() with nanosleep(0, 1ns). nanosleep() goes
through hrtimer_nanosleep() -> do_nanosleep(), which puts the task into
TASK_INTERRUPTIBLE and removes it from the run queue entirely. This
guarantees the scheduler picks the eat_cpu child, restoring the
context-switch guarantee the test requires. The 1ns duration is enough
to engage the hrtimer path while keeping the sleep effectively
instantaneous; the same race window between PMU overflow and context
switch is preserved.
Reported-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
Link: https://patch.msgid.link/20260903074036.63309-1-atrajeev@linux.ibm.com
|
|
Since the switch to generic entry, an interrupt from user mode
accounts user time twice: once in arch_interrupt_enter_prepare()
and again in arch_enter_from_user_mode(), which irqentry_enter()
invokes for the same interrupt:
arch_interrupt_enter_prepare()
account_cpu_user_entry() /* first */
irqentry_enter()
arch_enter_from_user_mode()
account_cpu_user_entry() /* second */
The second call charges the same interval again, because
account_cpu_user_entry() accumulates the time spent in user mode
since the last return to user space.
The two calls come from the GENERIC_ENTRY preparation series,
where each step was a no-op on its own. Commit 09a9d3a8499d
("powerpc: introduce arch_enter_from_user_mode") added the hook
with the user-time accounting in it, but nothing called it yet.
Commit 893082ac769b ("powerpc: Prepare for IRQ entry exit")
copied interrupt_enter_prepare() verbatim into entry-common.h as
arch_interrupt_enter_prepare(); that copy was equally unused, as
handlers still called interrupt_enter_prepare().
Commit bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature")
made both live. On the syscall side it did the full conversion:
system_call_exception() now accounts once through the hook via
syscall_enter_from_user_mode(), rather than calling
account_cpu_user_entry() directly. On the interrupt side it
switched the handler macros to arch_interrupt_enter_prepare()
followed by irqentry_enter(), which also runs the hook, but the
accounting in arch_interrupt_enter_prepare() was not removed to
match. The double accounting starts with that commit.
With CONFIG_VIRT_CPU_ACCOUNTING_NATIVE=y this roughly doubles the
reported user time of any workload that takes interrupts. The
other accounting modes compile account_cpu_user_entry() to an
empty stub, so they are not affected.
Remove the accounting from arch_interrupt_enter_prepare() and rely
on arch_enter_from_user_mode(), which already runs for both
syscalls and interrupts. The duplicate account_stolen_time() call
is removed the same way.
On a pseries LPAR a busy loop reports 6s user time in 3s elapsed
(~210% CPU) before the fix, and 3s (~105% CPU) after it:
$ python3 -c 'while True: pass' &
$ sleep 3; ps -p $! -o etime,time,pcpu
ELAPSED TIME %CPU
Before 00:03 00:00:06 210
After 00:03 00:00:03 105
A 50% load reports ~70% usr / 30% idle before the fix, and
~49% usr / 51% idle after it:
$ taskset -c 6 stress-ng --cpu 1 --cpu-load 50 &
$ mpstat -P 6 1
CPU %usr %idle
Before 6 69.74 30.26
After 6 48.51 50.50
Fixes: bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature")
Reviewed-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com>
Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
Reviewed-by: Amit Machhiwal <amachhiw@linux.ibm.com>
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Reviewed-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
Link: https://patch.msgid.link/20260904025831.3439809-1-aboorvad@linux.ibm.com
|
|
ufs_qcom_enable_lane_clks() and ufs_qcom_disable_lane_clks() currently
use clk_bulk_prepare_enable()/clk_bulk_disable_unprepare() on the entire
host->clks array obtained from devm_clk_bulk_get_all(). This array
contains all device clocks, not just lane symbol clocks.
Since the UFS core framework already manages the non-lane clocks via the
setup_clocks callback, the bulk enable/disable in the lane clock APIs
resulted in duplicate reference count increments on those shared
clocks. The extra enable counts were never balanced by a corresponding
disable from the framework's clock gating path, preventing the clock
reference counts from reaching zero and ultimately blocking CXO shutdown
during low-power states.
Fix this by restricting the lane clock APIs to only prepare/enable and
disable/unprepare the three lane symbol clocks (tx_lane0_sync_clk,
rx_lane0_sync_clk, rx_lane1_sync_clk), leaving the handling of all other
clocks to the UFS core framework. The lane clocks are now acquired
individually via devm_clk_get() instead of being looked up in the bulk
clock array.
Signed-off-by: Nitin Rawat <nitin.rawat@oss.qualcomm.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Link: https://patch.msgid.link/20260909053944.2827968-1-nitin.rawat@oss.qualcomm.com
Signed-off-by: Martin K. Petersen (Oracle) <mkp@kernel.org>
|
|
it in ufshcd"
Bean Huo <beanhuo@iokpp.de> says:
The devfreq core has three users of the optional ->get_cur_freq()
callback. Two of them check the return value, the third one does not
and passes an uninitialized frequency to the transition notifiers when
the callback fails. Patch 1 fixes that.
Patch 2 writes down what a driver is expected to return from the
callback. Today this has to be found by reading the devfreq core.
Patch 3 records the frequency the controller starts at.
ufshcd_init_clocks() puts the controller at its highest frequency, but
nothing writes that down, so clk_scaling.target_freq stays 0 and
devfreq starts with previous_freq at 0 as well. With use_pm_opp this
makes ufshcd_devfreq_get_dev_status() report 0 Hz, the ondemand
governor then asks for the maximum frequency, and
ufshcd_devfreq_target() runs a full ufshcd_devfreq_scale() that holds
up the queue for up to a second only to set the same OPP and the same
gear again.
Patch 4 adds the ->get_cur_freq() callback to ufshcd. Without it the
cur_freq attribute shows the last frequency the governor selected,
which is wrong whenever the controller is scaled outside the governor,
for example after writing 0 to clkscale_enable.
The patches touch two subsystems. Patches 1 and 2 are for the devfreq
tree, patches 3 and 4 are for the SCSI tree. The two halves are
independent, at build time and at run time, and can be applied in
either order.
Patch was tested on a Radxa Dragon Q6A (1d84000.ufshc):
before "echo 0 > clkscale_enable": cur_freq 75000000, target_freq 75000000
after "echo 0 > clkscale_enable": cur_freq 300000000, target_freq 75000000
Without it both files report 75000000 and keep doing so for as long as
clock scaling stays disabled. A 4 GiB direct read after enabling clock
scaling again counted the transitions in trans_stat and attributed
time to the 300000000 state, so the frequency the callback returns is
one that devfreq recognises.
One thing to be aware of: devfreq_monitor_resume() copies
previous_freq from the callback, but it does not call
devfreq_update_status(). A frequency change made while the governor
was suspended therefore does not show up as a transition. That is how
devfreq behaves today and this series does not change it.
Link: https://patch.msgid.link/20260907192140.2701755-1-beanhuo@iokpp.de
Signed-off-by: Martin K. Petersen (Oracle) <mkp@kernel.org>
|
|
When a driver does not provide a ->get_cur_freq() callback, the cur_freq
sysfs attribute shows devfreq->previous_freq, which only tracks the scaling
that the governor itself did.
The UFS controller is also scaled outside the governor. The clearest
example is writing 0 to clkscale_enable: ufshcd_clkscale_enable_store()
sets the clocks to max_freq through ufshcd_devfreq_scale() and suspends the
governor, so devfreq_set_target() is never called. After that, cur_freq
keeps showing the last frequency the governor chose instead of the one the
controller runs at, and it does so as long as clock scaling stays disabled.
Add ufshcd_devfreq_get_cur_freq(). It reports clk_scaling.target_freq when
OPPs are used and the first clock's curr_freq otherwise, the same values
that ufshcd_devfreq_get_dev_status() reports.
Signed-off-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Avri Altman <avri.altman@sandisk.com>
Reviewed-by: Stanley Jhu <stanleyjhu@google.com>
Link: https://patch.msgid.link/20260907192140.2701755-5-beanhuo@iokpp.de
Signed-off-by: Martin K. Petersen (Oracle) <mkp@kernel.org>
|
|
ufshcd_init_clocks() puts the controller at its highest frequency, but
nothing writes that down. clk_scaling.target_freq stays 0, and
devfreq_dev_profile.initial_freq is never set, so devfreq->previous_freq is
0 as well.
With use_pm_opp this shows up in a few places. The target_freq attribute
reads 0 until the governor scales for the first time.
ufshcd_devfreq_get_dev_status() reports 0 Hz, which makes the ondemand
governor ask for the maximum frequency. ufshcd_devfreq_target() then
sees 0 != max and runs a full ufshcd_devfreq_scale(), which holds up the
queue for up to a second only to set the same OPP and the same gear
again. Without OPPs the frequency is not reported as 0, but
previous_freq is, and devfreq_update_status() then drops the first
time_in_state update.
Record the maximum frequency in ufshcd_devfreq_init() instead.
ufshcd_add_lus() runs after ufshcd_probe_hba() has geared up to
hba->max_pwr_info.info, so the clocks and the gear are both at their
maximum by the time we get here. The only difference is that the first
governor poll no longer redoes work that is already done. From the second
poll on nothing changes, because target_freq held the maximum frequency
there anyway.
That first scale also re-applied the gear that
ufshcd_vops_freq_to_gear_speed() maps the maximum frequency to, so it
quietly corrected the link if the OPP table and the gear negotiated at
probe disagreed. That does not happen any more. On ufs-qcom the two cannot
disagree, because ufs_qcom_negotiate_pwr_mode() clamps the gear through
ufshcd_negotiate_pwr_params() against the same controller capability the
OPP table is written from.
clki->max_freq is the right value in both modes.
ufshcd_parse_clock_min_max_freq() fills it from the highest OPP, and
ufshcd_clkscale_enable_store() already uses it the same way.
Suggested-by: Stanley Jhu <stanleyjhu@google.com>
Signed-off-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Stanley Jhu <stanleyjhu@google.com>
Link: https://patch.msgid.link/20260907192140.2701755-4-beanhuo@iokpp.de
Signed-off-by: Martin K. Petersen (Oracle) <mkp@kernel.org>
|
|
The representor vnic devlink health reporter is created and destroyed
along the representor netdev (un)load path, which is not serialized by
the devlink instance lock. Destroying the reporter from there triggers
a devl_assert_locked() splat on driver unbind:
WARNING: net/devlink/core.c:259 at devl_assert_locked+0x54/0x70, CPU#2: bash/3758
Modules linked in: mlx5_vdpa vringh vdpa mlx5_ib mlx5_fwctl mlx5_core ...
CPU: 2 UID: 0 PID: 3758 Comm: bash Tainted: G W 6.19.0+ #1 PREEMPT
Tainted: [W]=WARN
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), ...
RIP: 0010:devl_assert_locked+0x54/0x70
Call Trace:
<TASK>
devl_health_reporter_destroy+0x3a/0x1b0
mlx5e_vport_rep_unload+0x12d/0x2b0 [mlx5_core]
mlx5_eswitch_unregister_vport_reps+0x1b8/0x220 [mlx5_core]
? __esw_offloads_unload_rep+0x190/0x190 [mlx5_core]
? kernfs_remove_by_name_ns+0xc3/0xf0
device_release_driver_internal+0x3b2/0x560
unbind_store+0xce/0xf0
Move the reporter's lifecycle to the eswitch devlink port (un)register
paths, which are already serialized by the devlink instance lock, and
store the handle on mlx5_devlink_port. Use the port's mlx5_vport as the
reporter priv since the diagnose callback only needs a device handle and
a vport number, and mlx5_vport carries both and is initialized before
any representor driver probes.
Fixes: cf14af140a5a ("net/mlx5e: Add vnic devlink health reporter to representors")
Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20260906090700.3761260-1-tariqt@nvidia.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
scsi_cdl_enable() uses length fields returned by MODE SENSE to locate
the ATA feature mode page in a 64-byte stack buffer. A target can report
a total length shorter than its mode header and block descriptors. The
unsigned subtraction used for the MODE SELECT length can wrap, and the
separately computed buf_data can point beyond buf.
During automatic scan, enable is false, so the read-modify-write of
buf_data[4] can clear the low two bits of a target-selected
out-of-bounds stack byte. scsi_mode_select() can then copy up to 64
bytes from outside the buffer into the outgoing MODE SELECT payload,
disclosing stack contents to the target.
This is reachable while scanning a USB storage device that identifies as
an ATA device and advertises CDL support. No filesystem mount or
userspace access to the block device is required.
On upstream commit cee9395acd80 ("Linux 7.3-rc1"), a build-specific,
one-vCPU QEMU/Raw Gadget proof using QEMU-only multi-UDC allocator
sampling executed a fixed proof command inside the guest and created a
UID-0-owned marker during automatic enumeration, with KASLR and NX
enabled.
The issue was independently found during security research at Drivesec
S.r.l.
Cap the available length to the buffer size. Validate and consume the
mode header and block descriptor lengths before using the page, and
require the five bytes needed to access the CDL field.
Fixes: 1b22cfb14142 ("scsi: core: Allow enabling and disabling command duration limits")
Reported-by: Sashiko AI Review <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/linux-scsi/20260717192313.93D791F000E9@smtp.kernel.org/
Link: https://lore.kernel.org/linux-scsi/20260717222931.AC4EE1F000E9@smtp.kernel.org/
Link: https://lore.kernel.org/linux-scsi/df13ec87ac9b28e3b0a2d9eb26477e276ff0278a.camel@HansenPartnership.com/
Cc: stable@vger.kernel.org
Assisted-by: LLM
Co-developed-by: Pimen Flavian Dei (Drivesec S.r.l.) <fdei@drivesec.com>
Signed-off-by: Pimen Flavian Dei (Drivesec S.r.l.) <fdei@drivesec.com>
Signed-off-by: Alberto Carboneri (Drivesec S.r.l.) <acarboneri@drivesec.com>
Link: https://lore.kernel.org/linux-scsi/20260717192313.93D791F000E9@smtp.kernel.org/
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Link: https://patch.msgid.link/20260904135410.360314-1-acarboneri@drivesec.com
Signed-off-by: Martin K. Petersen (Oracle) <mkp@kernel.org>
|
|
Add a new debugfs file to inject corruptions of the data and/or reftag.
This will be used to detect that protection information and/or file
system checksumming can detect random bit errors or misplaced writes.
To use this echo the start LBA, number of logical blocks and type of
corruption into the new "corrupt" debugfs file for each scsi_debug
device.
For example:
echo lba=42,num=1,bit_errors=2 > /sys/kernel/debug/scsi_debug/1:0:0:0/corrupt
or
echo lba=2,num=4,reftag_adjust=8 > /sys/kernel/debug/scsi_debug/1:0:0:0/corrupt
The injection will be used by new xfstests test cases.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Link: https://patch.msgid.link/20260831064500.2576832-2-hch@lst.de
Signed-off-by: Martin K. Petersen (Oracle) <mkp@kernel.org>
|
|
leaks"
Chandrakanth Patil <chandrakanth.patil@broadcom.com> says:
This series contains a set of fixes for the mpi3mr driver:
- Out-of-bounds accesses where values reported by the controller (device
handles, phy numbers, topology event entry counts, reply and sense
buffer addresses, event data lengths) are used to index arrays, derive
pointers or size copies without being checked first
- Out-of-bounds accesses in the BSG passthrough paths, from a request size
held in too narrow a variable and from a copy made without checking the
payload holds that much data
- Target device reference leaks and an I/O block counter leak on error and
teardown paths, the latter leaving a device blocked for I/O
- A response buffer copied back to user space without being zeroed first,
so its unwritten fields carry whatever the allocation held
- A use-after-free and a NULL dereference around the firmware event
workqueue during driver removal and PCI error recovery
Link: https://patch.msgid.link/20260825210411.301535-1-chandrakanth.patil@broadcom.com
Signed-off-by: Martin K. Petersen (Oracle) <mkp@kernel.org>
|
|
On a frozen PCI channel the driver unmaps the register window and frees the
interrupts, but leaves the firmware event workqueue running. An event
handler that is already in flight can still reach the register window after
it has been unmapped.
Stop event processing and drain the queue first, and start it again when
the channel resumes.
Fixes: 30bafe1774f0 ("scsi: mpi3mr: Support PCI Error Recovery callback handlers")
Signed-off-by: Chandrakanth Patil <chandrakanth.patil@broadcom.com>
Link: https://patch.msgid.link/20260825210411.301535-18-chandrakanth.patil@broadcom.com
Signed-off-by: Martin K. Petersen (Oracle) <mkp@kernel.org>
|
|
The enqueue path reads the workqueue pointer before taking the event lock,
while the remove path clears that pointer and destroys the workqueue while
holding it. The enqueue can therefore reach queue_work() after the
workqueue is gone.
Move the check inside the lock.
Fixes: 13ef29ea4aa0 ("scsi: mpi3mr: Add support for device add/remove event handling")
Signed-off-by: Chandrakanth Patil <chandrakanth.patil@broadcom.com>
Link: https://patch.msgid.link/20260825210411.301535-17-chandrakanth.patil@broadcom.com
Signed-off-by: Martin K. Petersen (Oracle) <mkp@kernel.org>
|
|
Memory allocated for the BSG diagnostic buffer status is not zeroed before
it is copied back to user space. Several fields in this structure are
reserved and never written by the driver. Use kzalloc() instead of
kmalloc() to zero out the allocated memory.
Fixes: 78b506984ebe ("scsi: mpi3mr: Add ioctl support for HDB")
Signed-off-by: Chandrakanth Patil <chandrakanth.patil@broadcom.com>
Link: https://patch.msgid.link/20260825210411.301535-16-chandrakanth.patil@broadcom.com
Signed-off-by: Martin K. Petersen (Oracle) <mkp@kernel.org>
|
|
The number of entries in a PCIe topology change event is used to walk the
entry array without being compared against the amount of event data that
was received.
Bound the entry count to the received event data before use.
Fixes: 8e653455547a ("scsi: mpi3mr: Add support for PCIe device event handling")
Signed-off-by: Chandrakanth Patil <chandrakanth.patil@broadcom.com>
Link: https://patch.msgid.link/20260825210411.301535-15-chandrakanth.patil@broadcom.com
Signed-off-by: Martin K. Petersen (Oracle) <mkp@kernel.org>
|
|
The size of the target device map buffer is held in a u16 while the number
of devices it is derived from is not bounded to fit. With enough devices
the size wraps, a short buffer is allocated, and the loop that fills it
writes past the end.
Do the calculation in size_t.
Fixes: fb428a2005fc ("scsi: mpi3mr: Fix issues in mpi3mr_get_all_tgt_info()")
Signed-off-by: Chandrakanth Patil <chandrakanth.patil@broadcom.com>
Link: https://patch.msgid.link/20260825210411.301535-14-chandrakanth.patil@broadcom.com
Signed-off-by: Martin K. Petersen (Oracle) <mkp@kernel.org>
|
|
The phy number that comes with a link change is used to index the node's
phy array without being compared against the number of phys allocated for
that node.
Check it first.
Fixes: 42fc9fee116f ("scsi: mpi3mr: Add helper functions to manage device's port")
Signed-off-by: Chandrakanth Patil <chandrakanth.patil@broadcom.com>
Link: https://patch.msgid.link/20260825210411.301535-13-chandrakanth.patil@broadcom.com
Signed-off-by: Martin K. Petersen (Oracle) <mkp@kernel.org>
|
|
The event data length from the reply is used as is, both when caching log
data and when sizing the buffer handed to the bottom half. A length larger
than the frame makes both of them read past the end of it.
Clamp the length to what the frame can hold.
Fixes: 13ef29ea4aa0 ("scsi: mpi3mr: Add support for device add/remove event handling")
Fixes: d0d19250ed81 ("scsi: mpi3mr: Rename log data save helper to reflect threaded/BH context")
Signed-off-by: Chandrakanth Patil <chandrakanth.patil@broadcom.com>
Link: https://patch.msgid.link/20260825210411.301535-12-chandrakanth.patil@broadcom.com
Signed-off-by: Martin K. Petersen (Oracle) <mkp@kernel.org>
|
