Re: [PATCH v4 6/7] ntfs3: add iomap tracepoints

[email protected]
Newsgroups org.kernel.vger.linux-trace-kernel
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Data truncation of 64-bit inode number to 32-bit unsigned long in trace events on 32-bit architectures.
--

--- Patch [6]: [PATCH v4 6/7] ntfs3: add iomap tracepoints ---
commit 6e265a35dba7936f19ebddff3d05806b6ce60f0a
Author: Baolin Liu <[email protected]>

ntfs3: add iomap tracepoints

This commit adds trace events for ntfs_iomap_begin() and ntfs_iomap_end()
to help observe iomap mapping activity.

> diff --git a/include/trace/events/ntfs3.h b/include/trace/events/ntfs3.h
> index e39765bc062f1..e0c0c44ce318f 100644
> --- a/include/trace/events/ntfs3.h
> +++ b/include/trace/events/ntfs3.h
[ ... ]
> +TRACE_EVENT(ntfs3_iomap_begin,
> +	TP_PROTO(struct inode *inode, loff_t offset, loff_t length,
> +		 unsigned int flags),
> +	TP_ARGS(inode, offset, length, flags),
> +	TP_STRUCT__entry(
> +		__field(unsigned long, ino)

[Severity: Low]
Is unsigned long wide enough for the inode number here? In struct inode,
i_ino is defined as a u64. On 32-bit architectures, won't assigning
inode->i_ino to an unsigned long silently truncate high inode numbers?
Could this be changed to use u64 instead?

> +		__field(loff_t, offset)
> +		__field(loff_t, length)
> +		__field(dev_t, dev)
> +		__field(unsigned int, flags)
> +	),
> +	TP_fast_assign(
> +		__entry->ino = inode->i_ino;
[ ... ]
> +TRACE_EVENT(ntfs3_iomap_end,
> +	TP_PROTO(struct inode *inode, loff_t pos, loff_t length,
> +		 ssize_t written, unsigned int flags),
> +	TP_ARGS(inode, pos, length, written, flags),
> +	TP_STRUCT__entry(
> +		__field(unsigned long, ino)

[Severity: Low]
Does this field also risk truncating the u64 inode->i_ino when running on
32-bit architectures?

> +		__field(loff_t, pos)
> +		__field(loff_t, length)
> +		__field(ssize_t, written)
> +		__field(dev_t, dev)
> +		__field(unsigned int, flags)
> +	),
> +	TP_fast_assign(
> +		__entry->ino = inode->i_ino;
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.