pNFS: Fix uninited ptr access in ext_tree_encode_commit

[PATCH] pNFS: Fix uninited ptr access in ext_tree_encode_commit

Posted by Sergey Bashirov 2 months, 3 weeks ago

Current implementation of the function assumes that the provided buffer
can always accommodate at least one encoded extent. This patch adds
handling of all theoretically possible values of be_prev, so that
ext_tree_encode_commit makes no assumptions about the provided buffer
size, and static checks pass without warnings.

Fixes: d84c4754f874 ("pNFS: Fix extent encoding in block/scsi layout")
Addresses-Coverity-ID: 1647611 ("Memory - illegal accesses  (UNINIT)")
Signed-off-by: Sergey Bashirov <sergeybashirov@gmail.com>
---
 fs/nfs/blocklayout/extent_tree.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/blocklayout/extent_tree.c b/fs/nfs/blocklayout/extent_tree.c
index 315949a7e92d..adc1fe190cfc 100644
--- a/fs/nfs/blocklayout/extent_tree.c
+++ b/fs/nfs/blocklayout/extent_tree.c
@@ -585,7 +585,7 @@ static int
 ext_tree_encode_commit(struct pnfs_block_layout *bl, __be32 *p,
 		size_t buffer_size, size_t *count, __u64 *lastbyte)
 {
-	struct pnfs_block_extent *be, *be_prev;
+	struct pnfs_block_extent *be, *be_prev = NULL;
 	int ret = 0;
 
 	spin_lock(&bl->bl_ext_lock);
@@ -611,10 +611,13 @@ ext_tree_encode_commit(struct pnfs_block_layout *bl, __be32 *p,
 	if (!ret) {
 		*lastbyte = (bl->bl_lwb != 0) ? bl->bl_lwb - 1 : U64_MAX;
 		bl->bl_lwb = 0;
-	} else {
+	} else if (be_prev) {
 		*lastbyte = be_prev->be_f_offset + be_prev->be_length;
 		*lastbyte <<= SECTOR_SHIFT;
 		*lastbyte -= 1;
+	} else {
+		/* Buffer too small even for one extent, count is zero */
+		*lastbyte = U64_MAX;
 	}
 	spin_unlock(&bl->bl_ext_lock);
 
-- 
2.43.0

Re: [PATCH] pNFS: Fix uninited ptr access in ext_tree_encode_commit

Posted by Dan Carpenter 2 months, 3 weeks ago

On Thu, Jul 17, 2025 at 05:34:04PM +0300, Sergey Bashirov wrote:
> Current implementation of the function assumes that the provided buffer
> can always accommodate at least one encoded extent. This patch adds
> handling of all theoretically possible values of be_prev, so that
> ext_tree_encode_commit makes no assumptions about the provided buffer
> size, and static checks pass without warnings.
> 
> Fixes: d84c4754f874 ("pNFS: Fix extent encoding in block/scsi layout")
> Addresses-Coverity-ID: 1647611 ("Memory - illegal accesses  (UNINIT)")
> Signed-off-by: Sergey Bashirov <sergeybashirov@gmail.com>
> ---

Generally, we wouldn't put a Fixes tag here because it's just silencing
a false positive.  But also putting a Fixes tag is fine.  Thanks!

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

regards,
dan carpenter

Re: [PATCH] pNFS: Fix uninited ptr access in ext_tree_encode_commit

Posted by Antonio Quartulli 2 months, 3 weeks ago

On 17/07/2025 16:34, Sergey Bashirov wrote:
> Current implementation of the function assumes that the provided buffer
> can always accommodate at least one encoded extent. This patch adds
> handling of all theoretically possible values of be_prev, so that
> ext_tree_encode_commit makes no assumptions about the provided buffer
> size, and static checks pass without warnings.
> 
> Fixes: d84c4754f874 ("pNFS: Fix extent encoding in block/scsi layout")
> Addresses-Coverity-ID: 1647611 ("Memory - illegal accesses  (UNINIT)")
> Signed-off-by: Sergey Bashirov <sergeybashirov@gmail.com>

Acked-by: Antonio Quartulli <antonio@mandelbit.com>

Thanks a lot!
Regards,


-- 
Antonio Quartulli

CEO and Co-Founder
Mandelbit Srl
https://www.mandelbit.com