| Message ID | 20241217225811.2437150-6-mtodorovac69@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
There's a fix for this already in the for-next branch: https://github.com/btrfs/linux/commit/1a287050962c6847fa4918d6b2a0f4cee35c6943 On 17/12/24 22:58, Mirsad Todorovac wrote: > > > The static analyser tool gave the following advice: > > ./fs/btrfs/ioctl.c:4987:9-16: WARNING opportunity for kmemdup > > 4986 if (!iov) { > → 4987 iov = kmalloc(sizeof(struct iovec) * args.iovcnt, GFP_NOFS); > 4988 if (!iov) { > 4989 unlock_extent(io_tree, start, lockend, &cached_state); > 4990 btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED); > 4991 ret = -ENOMEM; > 4992 goto out_acct; > 4993 } > 4994 > → 4995 memcpy(iov, iovstack, sizeof(struct iovec) * args.iovcnt); > 4996 } > > Replacing kmalloc() + memcpy() with kmemdump() doesn't change semantics. > Original code works without fault, so this is not a bug fix but proposed improvement. > > Link: https://urldefense.com/v3/__https://lwn.net/Articles/198928/__;!!Bt8RZUm9aw!4OVzQmIUbyH-UGdUwMAL582hR4Q-7HN2fn9IpyxeA1T8qrcC8RdBVz4xuL4m35_kksUllAi6OmdbRehcFpwfHw$ > Fixes: 34310c442e175 ("btrfs: add io_uring command for encoded reads (ENCODED_READ ioctl)") > Cc: Chris Mason <clm@fb.com> > Cc: Josef Bacik <josef@toxicpanda.com> > Cc: David Sterba <dsterba@suse.com> > Cc: linux-btrfs@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Mirsad Todorovac <mtodorovac69@gmail.com> > --- > v1: > initial version. > > fs/btrfs/ioctl.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > index 3af8bb0c8d75..c2f769334d3c 100644 > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -4984,15 +4984,13 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue > * undo this. > */ > if (!iov) { > - iov = kmalloc(sizeof(struct iovec) * args.iovcnt, GFP_NOFS); > + iov = kmemdup(iovstack, sizeof(struct iovec) * args.iovcnt, GFP_NOFS); > if (!iov) { > unlock_extent(io_tree, start, lockend, &cached_state); > btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED); > ret = -ENOMEM; > goto out_acct; > } > - > - memcpy(iov, iovstack, sizeof(struct iovec) * args.iovcnt); > } > > count = min_t(u64, iov_iter_count(&iter), disk_io_size);
Hi, Yes, I see you have the prior work and I have duplicated your work. Apology for the inconvenience. Best regards, Mirsad Todorovac On 12/18/24 11:32, Mark Harmstone wrote: > There's a fix for this already in the for-next branch: > https://github.com/btrfs/linux/commit/1a287050962c6847fa4918d6b2a0f4cee35c6943 > > On 17/12/24 22:58, Mirsad Todorovac wrote: >>> >> The static analyser tool gave the following advice: >> >> ./fs/btrfs/ioctl.c:4987:9-16: WARNING opportunity for kmemdup >> >> 4986 if (!iov) { >> → 4987 iov = kmalloc(sizeof(struct iovec) * args.iovcnt, GFP_NOFS); >> 4988 if (!iov) { >> 4989 unlock_extent(io_tree, start, lockend, &cached_state); >> 4990 btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED); >> 4991 ret = -ENOMEM; >> 4992 goto out_acct; >> 4993 } >> 4994 >> → 4995 memcpy(iov, iovstack, sizeof(struct iovec) * args.iovcnt); >> 4996 } >> >> Replacing kmalloc() + memcpy() with kmemdump() doesn't change semantics. >> Original code works without fault, so this is not a bug fix but proposed improvement. >> >> Link: https://urldefense.com/v3/__https://lwn.net/Articles/198928/__;!!Bt8RZUm9aw!4OVzQmIUbyH-UGdUwMAL582hR4Q-7HN2fn9IpyxeA1T8qrcC8RdBVz4xuL4m35_kksUllAi6OmdbRehcFpwfHw$ >> Fixes: 34310c442e175 ("btrfs: add io_uring command for encoded reads (ENCODED_READ ioctl)") >> Cc: Chris Mason <clm@fb.com> >> Cc: Josef Bacik <josef@toxicpanda.com> >> Cc: David Sterba <dsterba@suse.com> >> Cc: linux-btrfs@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Mirsad Todorovac <mtodorovac69@gmail.com> >> --- >> v1: >> initial version. >> >> fs/btrfs/ioctl.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c >> index 3af8bb0c8d75..c2f769334d3c 100644 >> --- a/fs/btrfs/ioctl.c >> +++ b/fs/btrfs/ioctl.c >> @@ -4984,15 +4984,13 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue >> * undo this. >> */ >> if (!iov) { >> - iov = kmalloc(sizeof(struct iovec) * args.iovcnt, GFP_NOFS); >> + iov = kmemdup(iovstack, sizeof(struct iovec) * args.iovcnt, GFP_NOFS); >> if (!iov) { >> unlock_extent(io_tree, start, lockend, &cached_state); >> btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED); >> ret = -ENOMEM; >> goto out_acct; >> } >> - >> - memcpy(iov, iovstack, sizeof(struct iovec) * args.iovcnt); >> } >> >> count = min_t(u64, iov_iter_count(&iter), disk_io_size); >
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 3af8bb0c8d75..c2f769334d3c 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -4984,15 +4984,13 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue * undo this. */ if (!iov) { - iov = kmalloc(sizeof(struct iovec) * args.iovcnt, GFP_NOFS); + iov = kmemdup(iovstack, sizeof(struct iovec) * args.iovcnt, GFP_NOFS); if (!iov) { unlock_extent(io_tree, start, lockend, &cached_state); btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED); ret = -ENOMEM; goto out_acct; } - - memcpy(iov, iovstack, sizeof(struct iovec) * args.iovcnt); } count = min_t(u64, iov_iter_count(&iter), disk_io_size);
The static analyser tool gave the following advice: ./fs/btrfs/ioctl.c:4987:9-16: WARNING opportunity for kmemdup 4986 if (!iov) { → 4987 iov = kmalloc(sizeof(struct iovec) * args.iovcnt, GFP_NOFS); 4988 if (!iov) { 4989 unlock_extent(io_tree, start, lockend, &cached_state); 4990 btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED); 4991 ret = -ENOMEM; 4992 goto out_acct; 4993 } 4994 → 4995 memcpy(iov, iovstack, sizeof(struct iovec) * args.iovcnt); 4996 } Replacing kmalloc() + memcpy() with kmemdump() doesn't change semantics. Original code works without fault, so this is not a bug fix but proposed improvement. Link: https://lwn.net/Articles/198928/ Fixes: 34310c442e175 ("btrfs: add io_uring command for encoded reads (ENCODED_READ ioctl)") Cc: Chris Mason <clm@fb.com> Cc: Josef Bacik <josef@toxicpanda.com> Cc: David Sterba <dsterba@suse.com> Cc: linux-btrfs@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Mirsad Todorovac <mtodorovac69@gmail.com> --- v1: initial version. fs/btrfs/ioctl.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)